Merged
Conversation
|
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:
📝 WalkthroughWalkthroughAdds a GitHub Actions Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor TagPush as "Tag push (refs/tags/v*)"
participant GH as "GitHub Actions"
participant Repo as "Repository (checkout, package.json)"
participant Buildx as "Buildx / docker/build-push-action"
participant DockerHub as "DockerHub"
TagPush->>GH: trigger workflow
GH->>Repo: checkout code
GH->>Repo: derive version from GITHUB_REF
GH->>Repo: read devDependencies (sequelize, sequelize-cli, mysql2) via jq
GH->>Repo: copy package `.dockerignore` files to repo root
GH->>Buildx: setup QEMU & Buildx, login to DockerHub
GH->>Buildx: build daemon image (`packages/daemon/Dockerfile`, target=dev) -> tag :dev, :dev-${version}
Buildx->>DockerHub: push daemon tags
GH->>Buildx: build lambdas image (`packages/wallet-service/Dockerfile.dev`) -> tag :dev, :dev-${version}
Buildx->>DockerHub: push lambdas tags
GH->>Repo: remove root `.dockerignore`
GH->>Buildx: build migrator image (`db/Dockerfile.dev`, build-args pinned) -> tag :dev, :dev-${version}
Buildx->>DockerHub: push migrator tags
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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: 2
🧹 Nitpick comments (1)
.github/workflows/main.yml (1)
138-143: Usejq -erto fail fast if dependency keys are missing.Currently,
jq -rreturns the string"null"for missing keys, allowing the workflow to continue and fail later during the Docker build. Adding the-eflag makesjqexit with code 1 when a key is absent, catching the problem immediately.Suggested change
- echo "sequelize=$(jq -r '.devDependencies["sequelize"]' package.json)" >> "$GITHUB_OUTPUT" - echo "sequelize_cli=$(jq -r '.devDependencies["sequelize-cli"]' package.json)" >> "$GITHUB_OUTPUT" - echo "mysql2=$(jq -r '.devDependencies["mysql2"]' package.json)" >> "$GITHUB_OUTPUT" + echo "sequelize=$(jq -er '.devDependencies["sequelize"]' package.json)" >> "$GITHUB_OUTPUT" + echo "sequelize_cli=$(jq -er '.devDependencies["sequelize-cli"]' package.json)" >> "$GITHUB_OUTPUT" + echo "mysql2=$(jq -er '.devDependencies["mysql2"]' package.json)" >> "$GITHUB_OUTPUT"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/main.yml around lines 138 - 143, The workflow step "Extract dependency versions for migrator" (id: deps) currently uses jq -r when reading devDependencies, which returns "null" for missing keys and allows the job to continue; update each jq invocation in that step to use jq -er instead (for sequelize, sequelize-cli, and mysql2 echo lines) so jq exits with code 1 on missing keys and the workflow fails fast with a clear error when a dependency key is absent.
🤖 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/main.yml:
- Around line 157-158: The workflow currently copies other packages'
.dockerignore files into the repository root (the "Copy daemon .dockerignore to
root" and similar steps), leaving the root .dockerignore stateful so the
migrator build inherits the wallet-service rules; fix by ensuring the migrator
build uses its own ignore/context: update the migrator docker build step to use
packages/migrator as the build context (and --file to point to the Dockerfile)
so packages/migrator/.dockerignore is honored, or alternatively copy
packages/migrator/.dockerignore immediately before the migrator build (instead
of relying on prior cp steps); locate the cp steps and the migrator build step
in the workflow and make one of these changes.
- Around line 131-132: Update the GitHub Actions checkout step to use a
supported major version by replacing the pinned reference "actions/checkout@v3"
with a supported release such as "actions/checkout@v4" in the workflow step
named "Checkout code" (and any other occurrences of "actions/checkout@v3");
ensure the step still uses the same inputs and semantic behavior after the
version change so the job runs under the runner's supported action versions.
---
Nitpick comments:
In @.github/workflows/main.yml:
- Around line 138-143: The workflow step "Extract dependency versions for
migrator" (id: deps) currently uses jq -r when reading devDependencies, which
returns "null" for missing keys and allows the job to continue; update each jq
invocation in that step to use jq -er instead (for sequelize, sequelize-cli, and
mysql2 echo lines) so jq exits with code 1 on missing keys and the workflow
fails fast with a clear error when a dependency key is absent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: acbaa3bd-8b91-416b-9f91-44d4b5f3bd76
📒 Files selected for processing (1)
.github/workflows/main.yml
luislhl
reviewed
Mar 19, 2026
luislhl
reviewed
Mar 19, 2026
luislhl
reviewed
Mar 19, 2026
luislhl
reviewed
Mar 19, 2026
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/main.yml (1)
138-143: Use lockfile-resolved versions for migrator build args to ensure reproducible builds.Extracting versions from
package.jsonpasses semver ranges (e.g.,^6.37.2), which allows minor/patch updates on subsequent rebuilds of the same git tag, compromising reproducibility. Sinceyarn.lockexists, extract exact resolved versions from it instead. You can usejqto parse the lockfile or pipeyarn list --depth 0and extract versions before passing them as build args.This applies to both locations: lines 138-143 and 199-202.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/main.yml around lines 138 - 143, The current GitHub Action step with id "deps" extracts semver ranges from package.json (echo "sequelize=...") which yields non‑reproducible build args; change the extraction to pull the exact resolved versions from the lockfile (yarn.lock) or by running "yarn list --depth 0" and parsing the output, then echo those resolved versions into $GITHUB_OUTPUT (for example replace the current echo lines for sequelize, sequelize_cli, and mysql2 with commands that read yarn.lock or parse yarn list and emit the exact version strings); apply the same change to the second location that sets these build args so both use lockfile-resolved versions.
🤖 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/main.yml:
- Line 132: The workflow uses floating major tags for actions
(actions/checkout@v6, docker/setup-qemu-action@v4,
docker/setup-buildx-action@v4, docker/login-action@v4,
docker/build-push-action@v7); update each `uses:` entry to the corresponding
immutable commit SHA instead of the major tag (replace e.g. actions/checkout@v6
with actions/checkout@<full-commit-sha> and do the same for
docker/setup-qemu-action, docker/setup-buildx-action, docker/login-action, and
docker/build-push-action occurrences), verifying the SHAs from the respective
GitHub Action repos and ensuring all listed occurrences are changed.
---
Nitpick comments:
In @.github/workflows/main.yml:
- Around line 138-143: The current GitHub Action step with id "deps" extracts
semver ranges from package.json (echo "sequelize=...") which yields
non‑reproducible build args; change the extraction to pull the exact resolved
versions from the lockfile (yarn.lock) or by running "yarn list --depth 0" and
parsing the output, then echo those resolved versions into $GITHUB_OUTPUT (for
example replace the current echo lines for sequelize, sequelize_cli, and mysql2
with commands that read yarn.lock or parse yarn list and emit the exact version
strings); apply the same change to the second location that sets these build
args so both use lockfile-resolved versions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 62399f05-ff21-41ec-81f4-cbd6bfec734a
📒 Files selected for processing (1)
.github/workflows/main.yml
luislhl
reviewed
Mar 19, 2026
Floating major tags like @v6 can be moved by the action maintainer to point to any commit. If a maintainer's account is compromised, an attacker could retag majors to a malicious commit.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/main.yml (1)
35-35: Inconsistentactions/checkoutversion between jobs.The
testjob still usesactions/checkout@v3while the newdockerjob uses SHA-pinnedv6.0.2. The commit message mentions pinning action versions to SHA, but this line was not updated. Consider aligning both jobs to use the same SHA-pinned version for consistency and to avoid actionlint warnings about unsupported runner versions.♻️ Suggested fix
- name: Checkout code - uses: actions/checkout@v3 + uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/main.yml at line 35, Update the `uses: actions/checkout@v3` line in the test job to match the SHA-pinned checkout reference used by the docker job (the same `actions/checkout` version/pin, e.g., `actions/checkout@v6.0.2` or the exact SHA tag used in the docker job) so both jobs use the identical pinned action reference and avoid actionlint warnings.
🤖 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/main.yml:
- Around line 174-188: The .dockerignore entry '../../.github/' will be wrong
after the file is copied to the repo root by the "Copy lambdas .dockerignore to
root" step; update the pattern in packages/wallet-service/.dockerignore to
reference the repository-relative path (e.g. '.github/' or '/.github/') instead
of '../../.github/' so when Docker build uses root context (file:
packages/wallet-service/Dockerfile.dev) the .github directory is correctly
excluded.
---
Nitpick comments:
In @.github/workflows/main.yml:
- Line 35: Update the `uses: actions/checkout@v3` line in the test job to match
the SHA-pinned checkout reference used by the docker job (the same
`actions/checkout` version/pin, e.g., `actions/checkout@v6.0.2` or the exact SHA
tag used in the docker job) so both jobs use the identical pinned action
reference and avoid actionlint warnings.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 57ff96ed-708b-4430-b030-e3837ba270d0
📒 Files selected for processing (1)
.github/workflows/main.yml
The pattern ../../.github/ was relative to the package directory, but Docker interprets .dockerignore paths relative to the build context root. Since the file is copied to the repo root before builds, the correct pattern is .github/. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Aligns the test job's actions/checkout reference with the docker job by using the same SHA-pinned v6.0.2 commit. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
.github/workflows/main.yml (1)
138-143:⚠️ Potential issue | 🟠 MajorEnforce exact dependency versions for deterministic Docker builds.
At lines 141-143, semver ranges (e.g.,
^6.37.2) are extracted frompackage.jsonand passed asSEQUELIZE_VERSION,SEQUELIZE_CLI_VERSION, andMYSQL2_VERSIONto the migrator Docker build. Indb/Dockerfile.devline 62, these are installed viayarn add sequelize@"$SEQUELIZE_VERSION", which resolves to different versions on each rebuild—breaking image reproducibility for tagged releases.Add validation to ensure only exact semantic versions are passed to the Docker build:
Validate exact version format
- name: Extract dependency versions for migrator id: deps run: | echo "sequelize=$(jq -er '.devDependencies["sequelize"]' package.json)" >> "$GITHUB_OUTPUT" echo "sequelize_cli=$(jq -er '.devDependencies["sequelize-cli"]' package.json)" >> "$GITHUB_OUTPUT" echo "mysql2=$(jq -er '.devDependencies["mysql2"]' package.json)" >> "$GITHUB_OUTPUT" + for dep in sequelize sequelize_cli mysql2; do + v="$(grep "^${dep}=" "$GITHUB_OUTPUT" | cut -d= -f2-)" + [[ "$v" =~ ^[0-9]+\.[0-9]+\.[0-9]+([.-].*)?$ ]] || { + echo "Non-exact version for ${dep}: ${v}" >&2 + exit 1 + } + done🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/main.yml around lines 138 - 143, The workflow currently exports semver ranges into SEQUELIZE_VERSION, SEQUELIZE_CLI_VERSION, and MYSQL2_VERSION (step id "deps") which Dockerfile.dev then installs via yarn add sequelize@"$SEQUELIZE_VERSION"; update the "deps" step to validate that each extracted version is an exact semantic version (no ^, ~, >=, etc.) using a strict regex (e.g. ^[0-9]+\.[0-9]+\.[0-9]+(?:-[0-9A-Za-z.]+)?$) and fail the job with a clear message if any value does not match, then only export the validated values to GITHUB_OUTPUT so the Docker build receives guaranteed exact versions. Ensure the validation references the same variables SEQUELIZE_VERSION, SEQUELIZE_CLI_VERSION, and MYSQL2_VERSION and protects the yarn add lines in Dockerfile.dev from receiving ranged versions.
🤖 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/main.yml:
- Around line 190-197: Add a db/.dockerignore that excludes everything except
the db/ directory (so only files needed by db/Dockerfile.dev are sent in the
build context), and in .github/workflows/main.yml replace the current step that
removes a non-existent root .dockerignore with a step that copies
db/.dockerignore to the repository root (e.g. cp db/.dockerignore .dockerignore)
before the "Build and push migrator dev image" step so the
docker/build-push-action using file: db/Dockerfile.dev will use the lighter
context; reference the workflow step named "Remove stale .dockerignore before
migrator build" and the Dockerfile path db/Dockerfile.dev when making the
change.
---
Duplicate comments:
In @.github/workflows/main.yml:
- Around line 138-143: The workflow currently exports semver ranges into
SEQUELIZE_VERSION, SEQUELIZE_CLI_VERSION, and MYSQL2_VERSION (step id "deps")
which Dockerfile.dev then installs via yarn add sequelize@"$SEQUELIZE_VERSION";
update the "deps" step to validate that each extracted version is an exact
semantic version (no ^, ~, >=, etc.) using a strict regex (e.g.
^[0-9]+\.[0-9]+\.[0-9]+(?:-[0-9A-Za-z.]+)?$) and fail the job with a clear
message if any value does not match, then only export the validated values to
GITHUB_OUTPUT so the Docker build receives guaranteed exact versions. Ensure the
validation references the same variables SEQUELIZE_VERSION,
SEQUELIZE_CLI_VERSION, and MYSQL2_VERSION and protects the yarn add lines in
Dockerfile.dev from receiving ranged versions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f68c89ae-fe55-4249-9eb8-351fdedd67bd
📒 Files selected for processing (2)
.github/workflows/main.ymlpackages/wallet-service/.dockerignore
✅ Files skipped from review due to trivial changes (1)
- packages/wallet-service/.dockerignore
Instead of removing the stale .dockerignore before the migrator build, copy a dedicated db/.dockerignore that excludes everything except db/. This reduces the build context sent to the Docker daemon since the migrator Dockerfile only needs the db/ directory. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
It would only be needed if we were building for archs other than amd64.
luislhl
approved these changes
Mar 20, 2026
andreabadesso
approved these changes
Mar 20, 2026
github-project-automation
Bot
moved this from In Review (WIP)
to Waiting to be deployed
in Hathor Network
This was referenced Apr 13, 2026
Merged
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
3 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.
Motivation
The Wallet Lib has, since HathorNetwork/hathor-wallet-lib#909 , used a Wallet Service image to run its integration tests, but those images are being built manually.
Having them built with each new tag ensures the integration tests are executed on an updated version of the Wallet Service.
Acceptance Criteria
sync-daemon,ws-migratorandlambasimages.Notes
This PR proposes two tags: one for the latest
devand other for a pinned version likedev-v1.12.0.It will also be necessary to manually verify if the DockerHub credentials are available for this repository before the script can actually work.
Checklist
master, confirm this code is production-ready and can be included in future releases as soon as it gets mergedSummary by CodeRabbit