refactor(docker-ci): experimental build docker tar - #317
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 24 minutes and 39 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughDocker CI workflow restructured from single-job build-and-push to multi-job pipeline architecture using artifact-based image distribution. Docker Hub polling mechanism replaced with artifact loading. Action versions updated to latest. Workflow permissions expanded. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/wait-for-docker-image.yml (1)
1-35:⚠️ Potential issue | 🔴 CriticalFix the parameter mismatch in ci.yml and update the workflow name/documentation — the implementation no longer waits.
ci.yml calls
wait-for-docker-image.ymlbut passes inputimage(lines 17–22), while the workflow expectsartifact-name(line 13 of wait-for-docker-image.yml). This parameter mismatch will cause the workflow to fail due to missing the required input.Additionally, the workflow file, name, and job ID all still advertise "waiting" behavior, but the implementation just downloads an artifact and runs
docker load— no polling or backoff. The header comment (lines 1–6) is outdated, referencing exponential backoff and race conditions with Docker Hub, neither of which apply.Required fixes:
- In ci.yml, change
with: image:towith: artifact-name: docker-artifact(to match what wait-for-docker-image.yml expects)- Rename
wait-for-docker-image.ymltoload-docker-image.yml, update thename:field toLoad Docker Image, and rename the job fromwait-for-imagetoload-image- Update the header comment to describe the artifact-based flow instead of polling
- Update all workflow references: ci.yml (line 15), and confirm the docker-ci.yml reference uses the correct file extension (.yml, not .yaml on line 67)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/wait-for-docker-image.yml around lines 1 - 35, The workflow file currently named wait-for-docker-image.yml still advertises "wait" behavior and expects input inputs.artifact-name, but ci.yml calls it with with: image:, and the implementation simply downloads and loads a docker artifact; update ci.yml to pass with: artifact-name: docker-artifact (so it matches inputs.artifact-name), rename the workflow file to load-docker-image.yml and change its name: field to "Load Docker Image" and the job id from wait-for-image to load-image, revise the header comment to describe the artifact-based download-and-docker-load flow (remove references to exponential backoff and Docker Hub), and update any references to this workflow (ci.yml invocation and docker-ci.yml reference) to point to load-docker-image.yml with the correct .yml extension.
🧹 Nitpick comments (1)
.github/workflows/docker-ci.yml (1)
22-23: Minor:outputs.tagsis now a single tag — the plural name is misleading.
steps.meta.outputs.tagsis always one value (e.g.ryanmillard/img2num-dev:pr-17) after the rewrite on Lines 37–45. The downstreamIFS=','loop indeploy(Lines 93–97) only exists because of this plural naming. Consider renaming the output toimage_ref(ortag) and removing the split loop — it'll be one less footgun if someone later assumes they can stuff multiple tags in here.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/docker-ci.yml around lines 22 - 23, The outputs field steps.meta.outputs.tags is misleading because it contains a single image reference; rename this output to a singular name like image_ref or tag (update usages like clean_core_tag: ${{ steps.meta.outputs.tags }} to use the new key) and remove the downstream IFS=',' split loop in the deploy job (which iterates over the plural tags) so deploy consumes the single image_ref/tag directly; update any references to steps.meta.outputs.tags to steps.meta.outputs.image_ref (or tag) and simplify deploy accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/docker-ci.yml:
- Around line 47-62: Update the "Upload Image Artifact" step that uses
actions/upload-artifact@v4 (the step named "Upload Image Artifact", artifact
name docker-artifact, path /tmp/image.tar) to include a retention-days setting
(e.g., retention-days: 1) under with:, so the artifact is removed quickly after
the workflow run and avoids accumulating multi-GB tars for every PR; keep the
existing name and path values intact.
- Around line 102-114: The notify job currently always creates a new PR comment
in the "Create Docker comment" step which will duplicate on every push and fails
for forked PRs; update the step to first run peter-evans/find-comment (e.g., use
a step id like find-comment) to locate an existing "🐳 Docker image built and
tested!" comment and pass its comment-id to peter-evans/create-or-update-comment
so it updates in-place, and add a guard on the step (or job) such as if:
github.event.pull_request.head.repo.full_name == github.repository to skip
running the action for external forks (or alternatively move this logic into a
pull_request_target workflow if you accept its security trade-offs).
- Around line 89-100: The workflow unconditionally re-tags and pushes :latest
(the docker tag commands that use env.IMAGE_NAME and docker tag
local-image:latest) on every push event, which regresses latest on tag pushes;
change the "Load and Push" step so it only pushes the :latest tag when the push
is to the main branch (e.g. guard with a conditional test on github.ref ==
'refs/heads/main' or equivalent), and simplify the tag handling by using the
single tag output (needs.build.outputs.tags or steps.meta.outputs.tags) instead
of the IFS comma-split loop — tag and push the computed tag(s) as before, and
only perform the docker tag ... :latest and docker push ...:latest when on main.
- Around line 64-69: Update the reusable workflow call in the "test" job to
reference the correct filename and forward secrets: change the uses path from
./.github/workflows/ci.yaml to ./.github/workflows/ci.yml (correct extension)
and add secrets: inherit under the test job so that secrets from the caller
(e.g., ALGOLIA_APP_ID, ALGOLIA_API_KEY, ALGOLIA_INDEX_NAME used in ci.yml) are
forwarded to the reusable workflow.
---
Outside diff comments:
In @.github/workflows/wait-for-docker-image.yml:
- Around line 1-35: The workflow file currently named wait-for-docker-image.yml
still advertises "wait" behavior and expects input inputs.artifact-name, but
ci.yml calls it with with: image:, and the implementation simply downloads and
loads a docker artifact; update ci.yml to pass with: artifact-name:
docker-artifact (so it matches inputs.artifact-name), rename the workflow file
to load-docker-image.yml and change its name: field to "Load Docker Image" and
the job id from wait-for-image to load-image, revise the header comment to
describe the artifact-based download-and-docker-load flow (remove references to
exponential backoff and Docker Hub), and update any references to this workflow
(ci.yml invocation and docker-ci.yml reference) to point to
load-docker-image.yml with the correct .yml extension.
---
Nitpick comments:
In @.github/workflows/docker-ci.yml:
- Around line 22-23: The outputs field steps.meta.outputs.tags is misleading
because it contains a single image reference; rename this output to a singular
name like image_ref or tag (update usages like clean_core_tag: ${{
steps.meta.outputs.tags }} to use the new key) and remove the downstream IFS=','
split loop in the deploy job (which iterates over the plural tags) so deploy
consumes the single image_ref/tag directly; update any references to
steps.meta.outputs.tags to steps.meta.outputs.image_ref (or tag) and simplify
deploy accordingly.
🪄 Autofix (Beta)
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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7f62739b-139c-477d-9088-216c3145dfd6
📒 Files selected for processing (2)
.github/workflows/docker-ci.yml.github/workflows/wait-for-docker-image.yml
| - name: Build and Export to Tar | ||
| uses: docker/build-push-action@v5 | ||
| with: | ||
| context: . | ||
| file: Dockerfile.dev | ||
| push: true | ||
| tags: ${{ steps.meta.outputs.tags }} | ||
| cache-from: | | ||
| type=registry,ref=${{ steps.meta.outputs.cache_ref }} | ||
| type=registry,ref=${{ env.IMAGE_NAME }}:buildcache | ||
| cache-to: type=registry,ref=${{ steps.meta.outputs.cache_ref }},mode=max | ||
|
|
||
| tags: local-image:latest | ||
| outputs: type=docker,dest=/tmp/image.tar | ||
| # Local cache to speed up subsequent runs without needing registry access | ||
| cache-from: type=gha | ||
| cache-to: type=gha,mode=max | ||
|
|
||
| - name: Upload Image Artifact | ||
| uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: docker-artifact | ||
| path: /tmp/image.tar |
There was a problem hiding this comment.
Artifacts have no retention-days — every PR will keep a full image tar for up to 90 days.
actions/upload-artifact@v4 defaults to the repository's artifact retention (up to 90 days). Because this runs on every pull_request opened/synchronize/reopened, you'll accumulate one image tar per push per open PR, which is easily multi-GB over time. Since the artifact is only consumed by the test and deploy jobs in the same run, a very short retention is appropriate.
🛠️ Proposed fix
- name: Upload Image Artifact
uses: actions/upload-artifact@v4
with:
name: docker-artifact
path: /tmp/image.tar
+ retention-days: 1
+ if-no-files-found: error📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Build and Export to Tar | |
| uses: docker/build-push-action@v5 | |
| with: | |
| context: . | |
| file: Dockerfile.dev | |
| push: true | |
| tags: ${{ steps.meta.outputs.tags }} | |
| cache-from: | | |
| type=registry,ref=${{ steps.meta.outputs.cache_ref }} | |
| type=registry,ref=${{ env.IMAGE_NAME }}:buildcache | |
| cache-to: type=registry,ref=${{ steps.meta.outputs.cache_ref }},mode=max | |
| tags: local-image:latest | |
| outputs: type=docker,dest=/tmp/image.tar | |
| # Local cache to speed up subsequent runs without needing registry access | |
| cache-from: type=gha | |
| cache-to: type=gha,mode=max | |
| - name: Upload Image Artifact | |
| uses: actions/upload-artifact@v4 | |
| with: | |
| name: docker-artifact | |
| path: /tmp/image.tar | |
| - name: Build and Export to Tar | |
| uses: docker/build-push-action@v5 | |
| with: | |
| context: . | |
| file: Dockerfile.dev | |
| tags: local-image:latest | |
| outputs: type=docker,dest=/tmp/image.tar | |
| # Local cache to speed up subsequent runs without needing registry access | |
| cache-from: type=gha | |
| cache-to: type=gha,mode=max | |
| - name: Upload Image Artifact | |
| uses: actions/upload-artifact@v4 | |
| with: | |
| name: docker-artifact | |
| path: /tmp/image.tar | |
| retention-days: 1 | |
| if-no-files-found: error |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/docker-ci.yml around lines 47 - 62, Update the "Upload
Image Artifact" step that uses actions/upload-artifact@v4 (the step named
"Upload Image Artifact", artifact name docker-artifact, path /tmp/image.tar) to
include a retention-days setting (e.g., retention-days: 1) under with:, so the
artifact is removed quickly after the workflow run and avoids accumulating
multi-GB tars for every PR; keep the existing name and path values intact.
There was a problem hiding this comment.
why didn't docker-ci start?
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check workflow runs for PR `#317`
gh run list --repo Ryan-Millard/Img2Num --branch feat/docker_tar --limit 20
echo "---"
# Check PR details
gh pr view 317 --repo Ryan-Millard/Img2Num --json number,state,headRefName,baseRefName,author,createdAt,statusCheckRollup
echo "---"
# Check if there are any workflow run failures or pending
gh api repos/Ryan-Millard/Img2Num/actions/runs --jq '.workflow_runs[] | select(.head_branch == "feat/docker_tar") | {id, name, status, conclusion, event, created_at}' 2>/dev/null | head -60
Length of output: 3215
🏁 Script executed:
#!/bin/bash
# Get details for the docker-ci.yml workflow run failure
gh api repos/Ryan-Millard/Img2Num/actions/runs/24743315310/jobs --jq '.jobs[] | {id, name, status, conclusion, steps: [.steps[]? | {name, conclusion, number}]}'
echo "=== CI startup_failure ==="
gh api repos/Ryan-Millard/Img2Num/actions/runs/24743315280/jobs --jq '.jobs[] | {id, name, status, conclusion, steps: [.steps[]? | {name, conclusion, number}]}'
echo "=== Workflow run details for docker-ci ==="
gh api repos/Ryan-Millard/Img2Num/actions/runs/24743315310 --jq '{name, status, conclusion, event, workflow_id, html_url, jobs_url}'
Length of output: 374
🏁 Script executed:
#!/bin/bash
# Find and read ci.yaml
fd -e yaml -e yml ci .github/workflows/
echo "==="
cat .github/workflows/ci.yaml 2>/dev/null || cat .github/workflows/ci.yml 2>/dev/null
echo "=== wait-for-docker-image ==="
cat .github/workflows/wait-for-docker-image.yml 2>/dev/null || cat .github/workflows/wait-for-docker-image.yaml 2>/dev/null
Length of output: 3861
@Krasner, there are three distinct issues causing both failures:
1. Wrong filename — ci.yaml vs ci.yml (causes docker-ci's 0-job failure)
In docker-ci.yml, the test job references:
uses: ./.github/workflows/ci.yamlBut the actual file is .github/workflows/ci.yml (.yml, not .yaml). GitHub rejects the workflow at validation time before any jobs can even queue — hence the 0-second, 0-job failure.
2. ci.yml has no workflow_call trigger
Even if the filename were correct, ci.yml's on: block only has pull_request and push. A file must declare workflow_call in its triggers to be callable as a reusable workflow. Without it, docker-ci.yml's test job can never work.
3. ci.yml was NOT updated to match wait-for-docker-image.yml's new interface (causes CI's startup_failure)
ci.yml triggered independently via its own pull_request trigger and immediately hit a startup failure because its wait-for-image job still passes the old image: input:
# ci.yml — still using the OLD interface
uses: ./.github/workflows/wait-for-docker-image.yml
with:
image: ryanmillard/img2num-dev:pr-... # ← this input no longer existsBut wait-for-docker-image.yml was updated in this PR to remove image, max_attempts, and initial_delay_seconds, replacing them with the required artifact-name input. GitHub treats this as an invalid input/missing required input → startup_failure.
Additionally, ci.yml's lint, cmake-build, build-react-app, and build-docs jobs all pull the container image from Docker Hub (ryanmillard/img2num-dev:pr-N), but this PR's new flow never pushes to Docker Hub on PRs anymore — so those jobs would also fail even if they started.
What needs to be fixed in ci.yml
ci.yml needs a fairly significant rework to support the new artifact-based approach:
- Add
workflow_callwith anartifact-nameinput to itson:block - Replace the
wait-for-imagejob's call towait-for-docker-image.ymlto passartifact-nameinstead ofimage - Update all downstream jobs that use Docker Hub image references in their
container:blocks — they need to either uselocal-image:latest(after the artifact is loaded) or restructure how the image is made available
Here's a rough sketch of the changes needed:
on:
pull_request:
branches: [main]
push:
branches: [main]
workflow_call: # ← add this
inputs:
artifact-name:
required: true
type: string wait-for-image:
uses: ./.github/workflows/wait-for-docker-image.yml
with:
artifact-name: ${{ inputs.artifact-name }} # ← pass through the artifactThe container image references are the harder part — container: fields on jobs pull from a registry, they can't directly use a locally loaded image on a different runner. This likely means the architecture needs further thought (e.g., running the lint/build steps in a single job that first loads the tar, or using a shared runner cache).
🧠 Learnings used
Learnt from: Ryan-Millard
Repo: Ryan-Millard/Img2Num PR: 308
File: Dockerfile.dev:96-97
Timestamp: 2026-04-11T22:21:35.244Z
Learning: In Ryan-Millard/Img2Num, Dockerfile.dev is a development-only image (not production). Running as root is acceptable and intentional in this image because: (1) pnpm is installed under /root/.local/share/pnpm, (2) /usr/src/app is a host-mounted volume where UID mismatches cause permission issues, and (3) dev workflows require root-level flexibility. Non-root USER suggestions for this Dockerfile should be avoided or carefully qualified.
Learnt from: Ryan-Millard
Repo: Ryan-Millard/Img2Num PR: 0
File: :0-0
Timestamp: 2026-04-01T20:45:20.732Z
Learning: In the Ryan-Millard/Img2Num repository, a PR body containing generic boilerplate such as "This contribution was made as part of an open-source documentation improvement initiative. All changes are meant to be helpful additions to the project." is a strong signal of an AI-generated slop PR and should be explicitly flagged during review.
There was a problem hiding this comment.
still getting .github/workfows/docker-ci.yml Failure
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Latest workflow runs on the branch
gh run list --repo Ryan-Millard/Img2Num --branch feat/docker_tar --limit 10
echo "=== latest docker-ci run details ==="
gh api repos/Ryan-Millard/Img2Num/actions/runs \
--jq '.workflow_runs[] | select(.head_branch == "feat/docker_tar") | {id, name, status, conclusion, event, created_at, html_url}' 2>/dev/null | head -80
echo "=== current ci.yml ==="
cat .github/workflows/ci.yml 2>/dev/null || cat .github/workflows/ci.yaml 2>/dev/null
echo "=== current docker-ci.yml ==="
cat .github/workflows/docker-ci.yml
echo "=== current wait-for-docker-image.yml ==="
cat .github/workflows/wait-for-docker-image.yml
Length of output: 11042
@Krasner, found the issue. In ci.yml, the expression is missing its $ prefix — GitHub Actions rejects the file at parse time, and since docker-ci.yml calls ci.yml, the validation error cascades and kills docker-ci.yml at 0 seconds too.
Root cause — missing $ in ci.yml
In the wait-for-image job of ci.yml:
# ❌ WRONG — not a valid GitHub Actions expression
artifact-name: {{ inputs.artifact-name }}
# ✅ CORRECT
artifact-name: ${{ inputs.artifact-name }}Fix that single character and both 0s failures should resolve.
Secondary issue — container images still point to Docker Hub
After the syntax fix, the lint, cmake-build, build-react-app, and build-docs jobs in ci.yml will still fail at runtime because they all pull their container image from Docker Hub:
container:
image: ryanmillard/img2num-dev:pr-317 # ← never pushed in the new flowBut with the new tar-based approach, the image is only loaded locally on the runner that runs wait-for-docker-image.yml. Each downstream job (lint, cmake-build, etc.) spins up a separate runner and tries to pull from Docker Hub — where the pr-317 tag no longer exists.
This is the deeper architectural problem. The options are:
| Option | Trade-off |
|---|---|
| Push a temporary PR tag to Docker Hub during PRs (revert to old flow for container jobs) | Simple, but re-adds the registry push step |
| Collapse all container-dependent jobs into a single job that loads the tar first | Loses parallelism |
| Publish to a private registry / GHCR and pull from there | Clean, but needs GHCR setup |
Use services: + a local registry container to share the image across jobs |
Complex but avoids Docker Hub |
The quickest unblocking fix is to push the image to GHCR or a temporary Docker Hub tag so the container jobs can pull it — the current architecture change (tar artifact) is fundamentally incompatible with container: image: in separate jobs.
🧠 Learnings used
Learnt from: Ryan-Millard
Repo: Ryan-Millard/Img2Num PR: 308
File: Dockerfile.dev:96-97
Timestamp: 2026-04-11T22:21:35.244Z
Learning: In Ryan-Millard/Img2Num, Dockerfile.dev is a development-only image (not production). Running as root is acceptable and intentional in this image because: (1) pnpm is installed under /root/.local/share/pnpm, (2) /usr/src/app is a host-mounted volume where UID mismatches cause permission issues, and (3) dev workflows require root-level flexibility. Non-root USER suggestions for this Dockerfile should be avoided or carefully qualified.
Learnt from: Ryan-Millard
Repo: Ryan-Millard/Img2Num PR: 0
File: :0-0
Timestamp: 2026-04-18T17:26:42.396Z
Learning: In the Ryan-Millard/Img2Num repository, the `.coderabbit.yaml` includes a `pre_merge_checks.custom_checks` entry named "CodeRabbit Config Needs Update" (mode: warning) that checks if a PR introduces new languages, linter config files, new file extensions, or new CI tooling without a corresponding update to `.coderabbit.yaml`.
Learnt from: Ryan-Millard
Repo: Ryan-Millard/Img2Num PR: 0
File: :0-0
Timestamp: 2026-04-01T20:45:20.732Z
Learning: In the Ryan-Millard/Img2Num repository, a PR body containing generic boilerplate such as "This contribution was made as part of an open-source documentation improvement initiative. All changes are meant to be helpful additions to the project." is a strong signal of an AI-generated slop PR and should be explicitly flagged during review.
There was a problem hiding this comment.
still getting a Failure error
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
echo "=== Latest runs on feat/docker_tar ==="
gh run list --repo Ryan-Millard/Img2Num --branch feat/docker_tar --limit 10
echo ""
echo "=== Most recent docker-ci run details ==="
LATEST_RUN=$(gh api repos/Ryan-Millard/Img2Num/actions/runs \
--jq '[.workflow_runs[] | select(.head_branch == "feat/docker_tar")] | sort_by(.created_at) | reverse | .[0] | {id, name, status, conclusion, event, created_at, html_url}' 2>/dev/null)
echo "$LATEST_RUN"
LATEST_ID=$(gh api repos/Ryan-Millard/Img2Num/actions/runs \
--jq '[.workflow_runs[] | select(.head_branch == "feat/docker_tar")] | sort_by(.created_at) | reverse | .[0].id' 2>/dev/null)
echo ""
echo "=== Jobs for run $LATEST_ID ==="
gh api repos/Ryan-Millard/Img2Num/actions/runs/$LATEST_ID/jobs \
--jq '.jobs[] | {id, name, status, conclusion, started_at, completed_at}'
echo ""
echo "=== All runs (id, name, conclusion) ==="
gh api repos/Ryan-Millard/Img2Num/actions/runs \
--jq '.workflow_runs[] | select(.head_branch == "feat/docker_tar") | {id, name, status, conclusion, event, created_at}' 2>/dev/null
echo ""
echo "=== current ci.yml ==="
cat .github/workflows/ci.yml 2>/dev/null
echo ""
echo "=== current docker-ci.yml ==="
cat .github/workflows/docker-ci.yml
echo ""
echo "=== current wait-for-docker-image.yml ==="
cat .github/workflows/wait-for-docker-image.yml
Length of output: 11592
@Krasner, found two more syntax errors in ci.yml that are causing the 0s parse failures on both workflows.
Error 1 — with: on a regular job (lint)
with: is only valid on uses: (reusable workflow call) jobs. The lint job has runs-on: ubuntu-latest, making it a regular job — with: at the job level is not a valid key there and GitHub rejects the entire file.
# ❌ WRONG — regular jobs don't support `with:` at the job level
lint:
runs-on: ubuntu-latest
with: # ← invalid here
artifact-name: ${{ inputs.artifact-name }}Remove the with: block from lint. The artifact name is already consumed directly inside the step via ${{ inputs.artifact-name }}, so the with: block is redundant:
# ✅ CORRECT
lint:
runs-on: ubuntu-latest
# no `with:` here — steps below already use ${{ inputs.artifact-name }} directly
steps:
- name: Download Build Artifact
uses: actions/download-artifact@v4
with:
name: ${{ inputs.artifact-name }}
path: /tmpError 2 — double/nested with: in build-docs
# ❌ WRONG — two `with:` keys, inner one is orphaned
build-docs:
uses: ./.github/workflows/build-docs.yml
with:
with: # ← stray duplicate
artifact-name: ${{ inputs.artifact-name }}Fix:
# ✅ CORRECT
build-docs:
uses: ./.github/workflows/build-docs.yml
with:
artifact-name: ${{ inputs.artifact-name }}
secrets:
ALGOLIA_APP_ID: ${{ secrets.ALGOLIA_APP_ID }}
ALGOLIA_API_KEY: ${{ secrets.ALGOLIA_API_KEY }}
ALGOLIA_INDEX_NAME: ${{ secrets.ALGOLIA_INDEX_NAME }}Both errors are in ci.yml. Fixing them should allow the workflows to at least parse and queue jobs. After that, the next runtime concern will be whether cmake-build.yml, build-react-app.yml, and build-docs.yml themselves accept the artifact-name input — if they don't declare it in their workflow_call.inputs, those jobs will fail at startup too.
🧠 Learnings used
Learnt from: Ryan-Millard
Repo: Ryan-Millard/Img2Num PR: 308
File: Dockerfile.dev:96-97
Timestamp: 2026-04-11T22:21:35.244Z
Learning: In Ryan-Millard/Img2Num, Dockerfile.dev is a development-only image (not production). Running as root is acceptable and intentional in this image because: (1) pnpm is installed under /root/.local/share/pnpm, (2) /usr/src/app is a host-mounted volume where UID mismatches cause permission issues, and (3) dev workflows require root-level flexibility. Non-root USER suggestions for this Dockerfile should be avoided or carefully qualified.
Learnt from: Ryan-Millard
Repo: Ryan-Millard/Img2Num PR: 0
File: :0-0
Timestamp: 2026-04-18T17:26:42.396Z
Learning: In the Ryan-Millard/Img2Num repository, the `.coderabbit.yaml` includes a `pre_merge_checks.custom_checks` entry named "CodeRabbit Config Needs Update" (mode: warning) that checks if a PR introduces new languages, linter config files, new file extensions, or new CI tooling without a corresponding update to `.coderabbit.yaml`.
Learnt from: Ryan-Millard
Repo: Ryan-Millard/Img2Num PR: 0
File: :0-0
Timestamp: 2026-04-01T20:45:20.732Z
Learning: In the Ryan-Millard/Img2Num repository, a PR body containing generic boilerplate such as "This contribution was made as part of an open-source documentation improvement initiative. All changes are meant to be helpful additions to the project." is a strong signal of an AI-generated slop PR and should be explicitly flagged during review.
| # --- CALL DOWNSTREAM TEST --- | ||
| test: | ||
| needs: build | ||
| if: github.event_name == 'pull_request' && needs.build.result == 'success' | ||
|
|
||
| uses: ./.github/workflows/ci.yaml | ||
| with: | ||
| artifact-name: docker-artifact |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm the actual filename + extension of the downstream workflow, and whether it uses secrets.
fd -H -t f '^ci\.ya?ml$' .github/workflows
rg -nP 'secrets\.' .github/workflows/ci.y*ml || true
rg -nP '^\s*on:\s*$|workflow_call' .github/workflows/ci.y*ml || trueRepository: Ryan-Millard/Img2Num
Length of output: 269
Reusable workflow call: fix filename extension and add secrets: inherit.
Two issues need fixing:
- The file is named
ci.yml(notci.yaml). The current reference will fail with a "workflow file not found" error. ci.ymluses secrets (ALGOLIA_APP_ID,ALGOLIA_API_KEY,ALGOLIA_INDEX_NAMEat lines 93–95), which are not forwarded in the current call. Addsecrets: inheritto forward all secrets.
Fix
test:
needs: build
- uses: ./.github/workflows/ci.yaml
+ uses: ./.github/workflows/ci.yml
with:
artifact-name: docker-artifact
+ secrets: inherit📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # --- CALL DOWNSTREAM TEST --- | |
| test: | |
| needs: build | |
| if: github.event_name == 'pull_request' && needs.build.result == 'success' | |
| uses: ./.github/workflows/ci.yaml | |
| with: | |
| artifact-name: docker-artifact | |
| # --- CALL DOWNSTREAM TEST --- | |
| test: | |
| needs: build | |
| uses: ./.github/workflows/ci.yml | |
| with: | |
| artifact-name: docker-artifact | |
| secrets: inherit |
🧰 Tools
🪛 actionlint (1.7.12)
[error] 67-67: could not read reusable workflow file for "./.github/workflows/ci.yaml": open /home/jailuser/git/.github/workflows/ci.yaml: no such file or directory
(workflow-call)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/docker-ci.yml around lines 64 - 69, Update the reusable
workflow call in the "test" job to reference the correct filename and forward
secrets: change the uses path from ./.github/workflows/ci.yaml to
./.github/workflows/ci.yml (correct extension) and add secrets: inherit under
the test job so that secrets from the caller (e.g., ALGOLIA_APP_ID,
ALGOLIA_API_KEY, ALGOLIA_INDEX_NAME used in ci.yml) are forwarded to the
reusable workflow.
| - name: Load and Push | ||
| run: | | ||
| docker load --input /tmp/image.tar | ||
| # Tag and push all computed tags | ||
| IFS=',' read -ra ADDR <<< "${{ needs.build.outputs.tags }}" | ||
| for tag in "${ADDR[@]}"; do | ||
| docker tag local-image:latest "$tag" | ||
| docker push "$tag" | ||
| done | ||
| # Also push 'latest' if it's a main/tag push | ||
| docker tag local-image:latest ${{ env.IMAGE_NAME }}:latest | ||
| docker push ${{ env.IMAGE_NAME }}:latest |
There was a problem hiding this comment.
:latest is pushed unconditionally on every push event, including v* tag pushes.
The deploy job runs on github.event_name == 'push', which fires for both push:branches:[main] and push:tags:["v*"] (Lines 7–9). As written, Lines 99–100 will therefore re-tag :latest to whatever was just pushed — so releasing a hotfix for an older version (v1.2.4 after v2.x shipped) would silently regress :latest to the older build.
Gate the :latest push to main-branch pushes only:
🛠️ Proposed fix
- name: Load and Push
run: |
docker load --input /tmp/image.tar
- # Tag and push all computed tags
- IFS=',' read -ra ADDR <<< "${{ needs.build.outputs.tags }}"
- for tag in "${ADDR[@]}"; do
- docker tag local-image:latest "$tag"
- docker push "$tag"
- done
- # Also push 'latest' if it's a main/tag push
- docker tag local-image:latest ${{ env.IMAGE_NAME }}:latest
- docker push ${{ env.IMAGE_NAME }}:latest
+ # Push the computed tag (e.g. ryanmillard/img2num-dev:main or :v1.2.3)
+ docker tag local-image:latest "${{ needs.build.outputs.tags }}"
+ docker push "${{ needs.build.outputs.tags }}"
+ # Only refresh :latest on main-branch pushes, not on arbitrary version-tag pushes
+ if [[ "${GITHUB_REF}" == "refs/heads/main" ]]; then
+ docker tag local-image:latest "${{ env.IMAGE_NAME }}:latest"
+ docker push "${{ env.IMAGE_NAME }}:latest"
+ fiThis also removes the IFS=',' comma-split loop, which is dead complexity now that steps.meta.outputs.tags is computed as a single tag (Line 45). If you expect multi-tag support later, consider exposing the tag list as a JSON array output and iterating with jq instead of string splitting.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Load and Push | |
| run: | | |
| docker load --input /tmp/image.tar | |
| # Tag and push all computed tags | |
| IFS=',' read -ra ADDR <<< "${{ needs.build.outputs.tags }}" | |
| for tag in "${ADDR[@]}"; do | |
| docker tag local-image:latest "$tag" | |
| docker push "$tag" | |
| done | |
| # Also push 'latest' if it's a main/tag push | |
| docker tag local-image:latest ${{ env.IMAGE_NAME }}:latest | |
| docker push ${{ env.IMAGE_NAME }}:latest | |
| - name: Load and Push | |
| run: | | |
| docker load --input /tmp/image.tar | |
| # Push the computed tag (e.g. ryanmillard/img2num-dev:main or :v1.2.3) | |
| docker tag local-image:latest "${{ needs.build.outputs.tags }}" | |
| docker push "${{ needs.build.outputs.tags }}" | |
| # Only refresh :latest on main-branch pushes, not on arbitrary version-tag pushes | |
| if [[ "${GITHUB_REF}" == "refs/heads/main" ]]; then | |
| docker tag local-image:latest "${{ env.IMAGE_NAME }}:latest" | |
| docker push "${{ env.IMAGE_NAME }}:latest" | |
| fi |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/docker-ci.yml around lines 89 - 100, The workflow
unconditionally re-tags and pushes :latest (the docker tag commands that use
env.IMAGE_NAME and docker tag local-image:latest) on every push event, which
regresses latest on tag pushes; change the "Load and Push" step so it only
pushes the :latest tag when the push is to the main branch (e.g. guard with a
conditional test on github.ref == 'refs/heads/main' or equivalent), and simplify
the tag handling by using the single tag output (needs.build.outputs.tags or
steps.meta.outputs.tags) instead of the IFS comma-split loop — tag and push the
computed tag(s) as before, and only perform the docker tag ... :latest and
docker push ...:latest when on main.
| notify: | ||
| needs: [build, test] | ||
| if: github.event_name == 'pull_request' | ||
| runs-on: ubuntu-latest | ||
|
|
||
| steps: | ||
| - name: Delete PR images | ||
| run: | | ||
| PR_NUMBER=${{ github.event.pull_request.number }} | ||
|
|
||
| TOKEN=$(curl -s -X POST \ | ||
| -H "Content-Type: application/json" \ | ||
| -d '{"username":"${{ secrets.DOCKERHUB_USERNAME }}","password":"${{ secrets.DOCKERHUB_TOKEN }}"}' \ | ||
| https://hub.docker.com/v2/users/login/ | jq -r .token) | ||
|
|
||
| IMAGE="${{ env.IMAGE_NAME }}" | ||
|
|
||
| TAGS=( | ||
| "pr-${PR_NUMBER}" | ||
| "cache-pr-${PR_NUMBER}" | ||
| ) | ||
|
|
||
| for TAG in "${TAGS[@]}"; do | ||
| curl -s -X DELETE \ | ||
| -H "Authorization: JWT ${TOKEN}" \ | ||
| "https://hub.docker.com/v2/repositories/${IMAGE}/tags/${TAG}/" \ | ||
| || echo "Tag $TAG not found or already deleted" | ||
| done | ||
| - name: Create Docker comment | ||
| uses: peter-evans/create-or-update-comment@v4 | ||
| with: | ||
| issue-number: ${{ github.event.pull_request.number }} | ||
| body: | | ||
| ## 🐳 Docker image built and tested! | ||
| The image was verified in the downstream pipeline. | ||
| **Tag:** `${{ needs.build.outputs.clean_core_tag }}` |
There was a problem hiding this comment.
PR will accumulate a new "built and tested" comment on every push.
peter-evans/create-or-update-comment only creates a new comment when called with issue-number; to update an existing one you have to pair it with peter-evans/find-comment and pass the resulting comment-id so the existing comment is replaced. As written, every synchronize event on an open PR will append another duplicate "🐳 Docker image built and tested!" comment.
Additionally note that in public repositories this action does not work in pull_request workflows triggered by forks — any attempt returns "Resource not accessible by integration" because of token restrictions, so external-contributor PRs will get a failed notify job. If that matters for this repo, guard the step with if: github.event.pull_request.head.repo.full_name == github.repository or move to pull_request_target with its usual security caveats.
🛠️ Proposed fix — update-in-place pattern
notify:
needs: [build, test]
if: github.event_name == 'pull_request'
runs-on: ubuntu-latest
steps:
- - name: Create Docker comment
- uses: peter-evans/create-or-update-comment@v4
- with:
- issue-number: ${{ github.event.pull_request.number }}
- body: |
- ## 🐳 Docker image built and tested!
- The image was verified in the downstream pipeline.
- **Tag:** `${{ needs.build.outputs.clean_core_tag }}`
+ - name: Find existing Docker comment
+ uses: peter-evans/find-comment@v3
+ id: fc
+ with:
+ issue-number: ${{ github.event.pull_request.number }}
+ comment-author: 'github-actions[bot]'
+ body-includes: '🐳 Docker image built and tested'
+
+ - name: Create or update Docker comment
+ uses: peter-evans/create-or-update-comment@v4
+ with:
+ comment-id: ${{ steps.fc.outputs.comment-id }}
+ issue-number: ${{ github.event.pull_request.number }}
+ edit-mode: replace
+ body: |
+ ## 🐳 Docker image built and tested!
+ The image was verified in the downstream pipeline.
+ **Tag:** `${{ needs.build.outputs.clean_core_tag }}`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/docker-ci.yml around lines 102 - 114, The notify job
currently always creates a new PR comment in the "Create Docker comment" step
which will duplicate on every push and fails for forked PRs; update the step to
first run peter-evans/find-comment (e.g., use a step id like find-comment) to
locate an existing "🐳 Docker image built and tested!" comment and pass its
comment-id to peter-evans/create-or-update-comment so it updates in-place, and
add a guard on the step (or job) such as if:
github.event.pull_request.head.repo.full_name == github.repository to skip
running the action for external forks (or alternatively move this logic into a
pull_request_target workflow if you accept its security trade-offs).
|
in theory: we shall see if it works |
8ce774e to
ea80726
Compare
|
@Krasner, are you still working on this? |
|
Paused for now. Do you want me to continue on this? |
I'm not sure at all. Do you think it's a worthwhile addition? Please will you explain exactly what the plan is with this PR so I can understand. The idea I previously had for the artifacts was kind of made obsolete by the recent update that allows us to comment |
Yes this was just experimental to create the docker build as an artifact (.tar), which would then be used by downstream tasks. This wouldn't need GHCR or dockerhub since it would just push around the tar file. But you've already resolved the GHCR issues so this isn't needed anymore |
I think it is better to not have this because the current maintainer-only version is very manual and deliberate, which will make things safer for everyone in the long run. We've already dealt with people who are only interested in their own gain and having to fix a problem like that on a registry would be terrible. |
|
Yes I agree |
2 participants
Build docker artifact as .tar then use it downstream
What was changed & why
Fixes: #
Changes
Testing & Verification
Additional Resources
Summary by CodeRabbit