build: migrate from Make to just as command runner#3110
Conversation
Replace Makefile + 6 .mk include files with a single justfile. Update all CI workflows (setup-just action) and docs references. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Warning Rate limit exceeded
⌛ 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. 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughReplace Make-based CI and local tooling with Just: remove modular Makefiles, add a top-level Justfile and multiple .just modules, and update GitHub workflows and documentation to invoke Just recipes instead of Make targets. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 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 |
|
Claude finished @tac0turtle's task in 1m 51s —— View job Code Review:
|
|
The latest Buf updates on your PR. Results from workflow CI / buf-check (pull_request).
|
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Groups: build, test, proto, lint, codegen, run, tools. Uses --unsorted to preserve logical ordering within groups. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![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 (4)
.github/workflows/benchmark.yml (2)
58-61: Same unpinned action concern applies here.For consistency, this instance of
extractions/setup-just@v4should also be pinned to a commit SHA if you decide to pin the first occurrence.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/benchmark.yml around lines 58 - 61, The workflow uses the unpinned action reference "extractions/setup-just@v4" in the "Install just" step; replace that tag with the same commit SHA you pinned for the first occurrence so the action is fixed to a specific commit (i.e., change uses: extractions/setup-just@v4 to uses: extractions/setup-just@<COMMIT_SHA>), ensuring consistency across both occurrences while leaving the subsequent "Build binaries" run step unchanged.
26-29: Pin the setup-just action to a specific commit SHA for consistency and reproducibility.All other actions in this workflow are pinned to specific commit SHAs (e.g.,
actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2), butextractions/setup-justuses an unpinned version tag@v4on both lines 27 and 59. Version tags can be moved or updated unexpectedly, introducing non-deterministic behavior.Suggested change
- name: Install just - uses: extractions/setup-just@v4 + uses: extractions/setup-just@e33e0265a09d6d736e2ee1e0eb685ef1de4669ff # v4🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/benchmark.yml around lines 26 - 29, Replace the unpinned GitHub Action usage "extractions/setup-just@v4" with a specific commit SHA to make the workflow deterministic; locate the two occurrences of the action in the benchmark workflow (the "Install just" step and the second occurrence around line 59) and update their ref from `@v4` to the exact commit SHA for the desired v4 release (e.g., @<commit-sha>) so the action is pinned consistently.justfile (1)
229-259: Consider adding atestlinkrecipe for link validation.The documentation in
docs/CLAUDE.mdreferencesjust testlinkfor validating markdown links using lychee, but no such recipe is defined. Consider adding it to maintain parity with the documented workflow.Suggested recipe to add
# Validate links in markdown files using lychee [group('lint')] testlink: `@echo` "--> Validating links in markdown files" `@lychee` --config .lychee.toml '**/*.md' || lychee '**/*.md'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@justfile` around lines 229 - 259, Add a new just recipe named testlink under the existing group('lint') to match docs/CLAUDE.md; implement a recipe called testlink that echoes a validating message and invokes lychee with the .lychee.toml config falling back to a plain lychee run (use the same glob '**/*.md'), ensuring it follows the pattern used by other recipes in the lint group (see existing lint, lint-fix, vet recipes for style and placement)..github/workflows/test.yml (1)
33-36: Minor style inconsistency in step naming.This job uses implicit step syntax (
- uses:/- run:) without step names, while other jobs in this workflow use explicit- name:labels. Consider adding step names for consistency and better readability in GitHub Actions logs.Suggested improvement for consistency
- - uses: extractions/setup-just@v4 - - run: just deps + - name: Install just + uses: extractions/setup-just@v4 + - name: Run dependency update + run: just deps🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/test.yml around lines 33 - 36, The workflow has unnamed steps using "uses: extractions/setup-just@v4" and "run: just deps" which is inconsistent with other jobs that use explicit step names; add descriptive "name:" labels for those two steps (e.g., name: Setup just and name: Install deps) so the steps referencing the uses: extractions/setup-just@v4 and the run: just deps are named consistently with the existing named step "name: check for diff" to improve readability in logs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/CLAUDE.md`:
- Line 13: The docs reference a missing Just recipe: add a `testlink` recipe to
the justfile (or update docs to the existing recipe name) so `just testlink`
actually runs lychee; implement a recipe named `testlink` in the justfile that
installs/uses lychee (or invokes the existing link-check target) and exits
non-zero on failures, or change the documentation lines (13, 47, 54) to the
current command name if you prefer not to add a recipe—look for the justfile and
the existing link-check task to ensure consistent naming between the justfile
(recipe `testlink`) and the docs.
In `@justfile`:
- Around line 151-155: Remove the duplicate unconditional echo of "--> Verifying
Docker image exists locally..." and keep a single verification message only
inside the conditional that checks EV_NODE_IMAGE_REPO (the block using the test
of "${EV_NODE_IMAGE_REPO:-}" and the docker image inspect evstack:local-dev
invocation); i.e., delete or relocate the standalone `@echo` "--> Verifying Docker
image exists locally..." so the message is emitted only when the conditional
runs around the docker image inspect command.
---
Nitpick comments:
In @.github/workflows/benchmark.yml:
- Around line 58-61: The workflow uses the unpinned action reference
"extractions/setup-just@v4" in the "Install just" step; replace that tag with
the same commit SHA you pinned for the first occurrence so the action is fixed
to a specific commit (i.e., change uses: extractions/setup-just@v4 to uses:
extractions/setup-just@<COMMIT_SHA>), ensuring consistency across both
occurrences while leaving the subsequent "Build binaries" run step unchanged.
- Around line 26-29: Replace the unpinned GitHub Action usage
"extractions/setup-just@v4" with a specific commit SHA to make the workflow
deterministic; locate the two occurrences of the action in the benchmark
workflow (the "Install just" step and the second occurrence around line 59) and
update their ref from `@v4` to the exact commit SHA for the desired v4 release
(e.g., @<commit-sha>) so the action is pinned consistently.
In @.github/workflows/test.yml:
- Around line 33-36: The workflow has unnamed steps using "uses:
extractions/setup-just@v4" and "run: just deps" which is inconsistent with other
jobs that use explicit step names; add descriptive "name:" labels for those two
steps (e.g., name: Setup just and name: Install deps) so the steps referencing
the uses: extractions/setup-just@v4 and the run: just deps are named
consistently with the existing named step "name: check for diff" to improve
readability in logs.
In `@justfile`:
- Around line 229-259: Add a new just recipe named testlink under the existing
group('lint') to match docs/CLAUDE.md; implement a recipe called testlink that
echoes a validating message and invokes lychee with the .lychee.toml config
falling back to a plain lychee run (use the same glob '**/*.md'), ensuring it
follows the pattern used by other recipes in the lint group (see existing lint,
lint-fix, vet recipes for style and placement).
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
.github/workflows/benchmark.yml.github/workflows/dependabot-auto-fix.yml.github/workflows/docker-tests.yml.github/workflows/test.ymlCLAUDE.mdMakefileREADME.mdclient/crates/types/README.mddocs/CLAUDE.mddocs/guides/create-genesis.mddocs/guides/evm/single.mddocs/guides/gm-world.mddocs/guides/quick-start.mdjustfilepkg/cmd/README.mdscripts/build.mkscripts/proto.mkscripts/run.mkscripts/test.mkscripts/utils.mktools/local-da/README.mdtools/tools.mk
💤 Files with no reviewable changes (7)
- scripts/proto.mk
- scripts/utils.mk
- Makefile
- scripts/run.mk
- scripts/build.mk
- scripts/test.mk
- tools/tools.mk
| - `yarn build` - Build static documentation site | ||
| - `yarn preview` - Preview built site locally | ||
| - `make testlink` - Validate all links in markdown files using lychee | ||
| - `just testlink` - Validate all links in markdown files using lychee |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for testlink recipe in justfile
if [ -f "justfile" ]; then
echo "=== Checking justfile for testlink recipe ==="
grep -n "testlink" justfile || echo "No testlink recipe found"
echo ""
echo "=== All recipes in justfile ==="
grep -n "^[a-zA-Z_].*:" justfile | head -20
else
echo "justfile not found"
fiRepository: evstack/ev-node
Length of output: 863
🏁 Script executed:
#!/bin/bash
# Check docs/CLAUDE.md for references to testlink
echo "=== Checking docs/CLAUDE.md ==="
if [ -f "docs/CLAUDE.md" ]; then
grep -n "testlink" docs/CLAUDE.md
echo ""
echo "=== Context around line 13 ==="
sed -n '10,16p' docs/CLAUDE.md
else
echo "docs/CLAUDE.md not found"
fiRepository: evstack/ev-node
Length of output: 565
Add missing testlink recipe to justfile.
Documentation at lines 13, 47, and 54 instructs users to run just testlink for link validation using lychee, but this recipe is not defined in the justfile. Add the recipe or update the documentation to reflect the actual command.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/CLAUDE.md` at line 13, The docs reference a missing Just recipe: add a
`testlink` recipe to the justfile (or update docs to the existing recipe name)
so `just testlink` actually runs lychee; implement a recipe named `testlink` in
the justfile that installs/uses lychee (or invokes the existing link-check
target) and exits non-zero on failures, or change the documentation lines (13,
47, 54) to the current command name if you prefer not to add a recipe—look for
the justfile and the existing link-check task to ensure consistent naming
between the justfile (recipe `testlink`) and the docs.
justfile
Outdated
| @echo "--> Running Docker E2E tests" | ||
| @echo "--> Verifying Docker image exists locally..." | ||
| @if [ -z "${EV_NODE_IMAGE_REPO:-}" ] || [ "${EV_NODE_IMAGE_REPO:-}" = "ev-node" ]; then \ | ||
| echo "--> Verifying Docker image exists locally..."; \ | ||
| docker image inspect evstack:local-dev >/dev/null 2>&1 || (echo "ERROR: evstack:local-dev image not found. Run 'just docker-build' first." && exit 1); \ |
There was a problem hiding this comment.
Duplicate echo statement.
Lines 151-152 both echo messages about verifying Docker image existence, with line 154 being inside the conditional block. The first echo on line 152 runs unconditionally while line 154 runs conditionally.
Suggested fix
test-docker-e2e: docker-build-if-local
`@echo` "--> Running Docker E2E tests"
- `@echo` "--> Verifying Docker image exists locally..."
`@if` [ -z "${EV_NODE_IMAGE_REPO:-}" ] || [ "${EV_NODE_IMAGE_REPO:-}" = "ev-node" ]; then \
echo "--> Verifying Docker image exists locally..."; \
docker image inspect evstack:local-dev >/dev/null 2>&1 || (echo "ERROR: evstack:local-dev image not found. Run 'just docker-build' first." && exit 1); \
fi📝 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.
| @echo "--> Running Docker E2E tests" | |
| @echo "--> Verifying Docker image exists locally..." | |
| @if [ -z "${EV_NODE_IMAGE_REPO:-}" ] || [ "${EV_NODE_IMAGE_REPO:-}" = "ev-node" ]; then \ | |
| echo "--> Verifying Docker image exists locally..."; \ | |
| docker image inspect evstack:local-dev >/dev/null 2>&1 || (echo "ERROR: evstack:local-dev image not found. Run 'just docker-build' first." && exit 1); \ | |
| `@echo` "--> Running Docker E2E tests" | |
| `@if` [ -z "${EV_NODE_IMAGE_REPO:-}" ] || [ "${EV_NODE_IMAGE_REPO:-}" = "ev-node" ]; then \ | |
| echo "--> Verifying Docker image exists locally..."; \ | |
| docker image inspect evstack:local-dev >/dev/null 2>&1 || (echo "ERROR: evstack:local-dev image not found. Run 'just docker-build' first." && exit 1); \ |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@justfile` around lines 151 - 155, Remove the duplicate unconditional echo of
"--> Verifying Docker image exists locally..." and keep a single verification
message only inside the conditional that checks EV_NODE_IMAGE_REPO (the block
using the test of "${EV_NODE_IMAGE_REPO:-}" and the docker image inspect
evstack:local-dev invocation); i.e., delete or relocate the standalone `@echo`
"--> Verifying Docker image exists locally..." so the message is emitted only
when the conditional runs around the docker image inspect command.
auricom
left a comment
auricom
left a comment
There was a problem hiding this comment.
Better than make 😄
we could split the justfile into multiple small ones, like this : https://github.com/auricom/home-ops/blob/main/.justfile#L6-L10
but its not really needed atm
Root justfile now holds variables and imports. Recipe files: build, test, proto, lint, codegen, run, tools. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (5)
.just/proto.just (1)
12-12: Consider pinning the buf Docker image version.Using
bufbuild/buf:latestcan lead to inconsistent linting results across builds. Consider pinning to a specific version for reproducibility.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.just/proto.just at line 12, The Docker invocation uses the floating image tag "bufbuild/buf:latest" which can cause non-reproducible lint results; change the image reference in the Docker command (the line containing "docker run ... bufbuild/buf:latest lint") to a fixed buf release tag (or move the image tag into a variable and reference that) so the buf version is pinned for reproducible linting across builds..just/codegen.just (1)
5-5: Consider pinning mockery version for reproducible builds.Using
@latestcan lead to inconsistent builds if mockery releases breaking changes. Consider pinning to a specific version or using a version variable defined in the root justfile.♻️ Suggested fix
- go run github.com/vektra/mockery/v3@latest + go run github.com/vektra/mockery/v3@v3.1.0Alternatively, define a
mockery_versionvariable in the root justfile for easier updates.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.just/codegen.just at line 5, Replace the unstable use of "go run github.com/vektra/mockery/v3@latest" with a pinned version or a variable reference: either hardcode a specific tag (e.g., `@vX.Y.Z`) in the command or reference a root-level variable like mockery_version (e.g., github.com/vektra/mockery/v3@${mockery_version}) so builds are reproducible and upgrades are centralized; update the .just/codegen.just invocation to use that pinned version or variable and add the mockery_version variable to the root justfile..just/build.just (1)
20-31: Consider using dependencies to reduce duplication inbuild-all.The
build-allrecipe duplicates the build commands frombuild,build-evm, andbuild-da. Using Just's dependency system would eliminate this duplication and ensure consistency.♻️ Suggested refactor
# Build all ev-node binaries [group('build')] -build-all: +build-all: build build-evm build-da `@echo` "--> Building all ev-node binaries" - `@mkdir` -p {{ build_dir }} - `@echo` "--> Building testapp" - `@cd` apps/testapp && go build -ldflags "{{ ldflags }}" -o {{ build_dir }}/testapp . - `@echo` "--> Building evm" - `@cd` apps/evm && go build -ldflags "{{ ldflags }}" -o {{ build_dir }}/evm . - `@echo` "--> Building local-da" - `@cd` tools/local-da && go build -ldflags "{{ ldflags }}" -o {{ build_dir }}/local-da . `@echo` "--> All ev-node binaries built!"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.just/build.just around lines 20 - 31, The build-all recipe duplicates the commands in existing recipes; refactor it to declare dependencies on the individual recipes (e.g., build, build-evm, build-da or whatever the single-target recipes are named) instead of repeating their commands so Just will run those recipes in order; update the build-all target under group('build') to list those recipe names as prerequisites and remove the duplicated mkdir/cd/go build lines, ensuring any shared variables (like build_dir and ldflags) remain referenced by the depended-on recipes..just/lint.just (1)
8-9: Hadolint only checks one specific Dockerfile.The hadolint check is hardcoded to
test/docker/mockserv.Dockerfile. Other Dockerfiles in the repository (likeapps/testapp/Dockerfilereferenced indocker-build) won't be linted.♻️ Consider linting all Dockerfiles
`@echo` "--> Running hadolint" - `@hadolint` test/docker/mockserv.Dockerfile + `@fd` -e Dockerfile -x hadolint {}Or explicitly list all Dockerfiles that should be linted.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.just/lint.just around lines 8 - 9, The hadolint invocation is hardcoded to test/docker/mockserv.Dockerfile (the line invoking hadolint), so update the hadolint step to lint all Dockerfiles instead of a single path: either replace the hardcoded argument with a discovery command that finds all Dockerfiles (e.g., find or git ls-files patterns) and passes them to hadolint, or explicitly list every repo Dockerfile (including apps/testapp/Dockerfile and test/docker/mockserv.Dockerfile) in that hadolint invocation; modify the hadolint invocation line accordingly so it covers all Dockerfiles..just/test.just (1)
63-64: Cleanup runs even if tests fail.The
docker-cleanup-if-localcall on Line 64 executes regardless of whether the tests on Line 63 pass or fail. If tests fail, the recipe exits before reaching Line 64, but if there's an intention to always cleanup, consider using Just's[no-exit-message]attribute or shell traps for guaranteed cleanup.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.just/test.just around lines 63 - 64, The cleanup currently invoked by the separate line "@just docker-cleanup-if-local" may not run if the test command fails; change the recipe so cleanup is guaranteed by installing an EXIT trap before running the tests (e.g., add a line that runs "trap 'just docker-cleanup-if-local' EXIT" prior to the "@cd test/docker-e2e && go test -mod=readonly -failfast -v -tags='docker_e2e' -timeout=30m ./..." command and remove the standalone "@just docker-cleanup-if-local" line), or alternatively apply Just's "[no-exit-message]" attribute and wrap the test invocation to ensure the cleanup command always executes on exit; update the lines referencing the test command and the "@just docker-cleanup-if-local" invocation 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 @.just/build.just:
- Around line 8-9: The echo messages are inconsistent: one hardcodes
"build/testapp" while the other uses the template variable "{{ build_dir
}}/testapp"; update the first echo to use the same build_dir variable so both
lines reference "{{ build_dir }}/testapp" (edit the echo string in the `@echo`
that currently prints "Check the binary with: build/testapp" to use "{{
build_dir }}/testapp" instead).
In @.just/codegen.just:
- Around line 9-13: The deps recipe currently runs "go run scripts/tidy.go"
without the build tag, which fails because scripts/tidy.go has a //go:build tidy
constraint; update the deps recipe to invoke the script with the build tag (use
"go run -tags=tidy scripts/tidy.go") so it compiles like the tidy-all recipe and
includes the required -tags=tidy flag.
In @.just/proto.just:
- Around line 24-25: The rust-proto-check recipe deletes generated proto files
but then runs cargo build without setting EV_TYPES_FORCE_PROTO_GEN=1 so
rust-proto-gen won’t regenerate files and git diff will always fail; update the
rust-proto-check recipe (the block that runs the commands removing
client/crates/types/src/proto/*.rs and then "cd client/crates/types && cargo
build") to run cargo build with the env var set (e.g. prefix the build with
EV_TYPES_FORCE_PROTO_GEN=1 or export it before invoking cargo) so rust-proto-gen
is triggered during the check.
In @.just/test.just:
- Around line 57-62: Remove the duplicated echo and make the repo-name check
consistent with the docker-build-if-local logic: delete the repeated "@echo
\"--> Verifying Docker image exists locally...\"" and change the conditional
that currently compares EV_NODE_IMAGE_REPO to "ev-node" so it compares to
"evstack" (the same value used by docker-build-if-local); keep a single echo
before the conditional and ensure the docker image inspect line still checks
evstack:local-dev.
---
Nitpick comments:
In @.just/build.just:
- Around line 20-31: The build-all recipe duplicates the commands in existing
recipes; refactor it to declare dependencies on the individual recipes (e.g.,
build, build-evm, build-da or whatever the single-target recipes are named)
instead of repeating their commands so Just will run those recipes in order;
update the build-all target under group('build') to list those recipe names as
prerequisites and remove the duplicated mkdir/cd/go build lines, ensuring any
shared variables (like build_dir and ldflags) remain referenced by the
depended-on recipes.
In @.just/codegen.just:
- Line 5: Replace the unstable use of "go run
github.com/vektra/mockery/v3@latest" with a pinned version or a variable
reference: either hardcode a specific tag (e.g., `@vX.Y.Z`) in the command or
reference a root-level variable like mockery_version (e.g.,
github.com/vektra/mockery/v3@${mockery_version}) so builds are reproducible and
upgrades are centralized; update the .just/codegen.just invocation to use that
pinned version or variable and add the mockery_version variable to the root
justfile.
In @.just/lint.just:
- Around line 8-9: The hadolint invocation is hardcoded to
test/docker/mockserv.Dockerfile (the line invoking hadolint), so update the
hadolint step to lint all Dockerfiles instead of a single path: either replace
the hardcoded argument with a discovery command that finds all Dockerfiles
(e.g., find or git ls-files patterns) and passes them to hadolint, or explicitly
list every repo Dockerfile (including apps/testapp/Dockerfile and
test/docker/mockserv.Dockerfile) in that hadolint invocation; modify the
hadolint invocation line accordingly so it covers all Dockerfiles.
In @.just/proto.just:
- Line 12: The Docker invocation uses the floating image tag
"bufbuild/buf:latest" which can cause non-reproducible lint results; change the
image reference in the Docker command (the line containing "docker run ...
bufbuild/buf:latest lint") to a fixed buf release tag (or move the image tag
into a variable and reference that) so the buf version is pinned for
reproducible linting across builds.
In @.just/test.just:
- Around line 63-64: The cleanup currently invoked by the separate line "@just
docker-cleanup-if-local" may not run if the test command fails; change the
recipe so cleanup is guaranteed by installing an EXIT trap before running the
tests (e.g., add a line that runs "trap 'just docker-cleanup-if-local' EXIT"
prior to the "@cd test/docker-e2e && go test -mod=readonly -failfast -v
-tags='docker_e2e' -timeout=30m ./..." command and remove the standalone "@just
docker-cleanup-if-local" line), or alternatively apply Just's
"[no-exit-message]" attribute and wrap the test invocation to ensure the cleanup
command always executes on exit; update the lines referencing the test command
and the "@just docker-cleanup-if-local" invocation accordingly.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
.just/build.just.just/codegen.just.just/lint.just.just/proto.just.just/run.just.just/test.just.just/tools.justjustfile
🚧 Files skipped from review as they are similar to previous changes (1)
- justfile
| @echo " Check the version with: build/testapp version" | ||
| @echo " Check the binary with: {{ build_dir }}/testapp" |
There was a problem hiding this comment.
Inconsistent path reference in echo message.
Line 8 uses a hardcoded path build/testapp while Line 9 correctly uses {{ build_dir }}/testapp. This could cause confusion if build_dir is changed.
🔧 Proposed fix
`@echo` "--> Testapp CLI Built!"
- `@echo` " Check the version with: build/testapp version"
+ `@echo` " Check the version with: {{ build_dir }}/testapp version"
`@echo` " Check the binary with: {{ build_dir }}/testapp"📝 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.
| @echo " Check the version with: build/testapp version" | |
| @echo " Check the binary with: {{ build_dir }}/testapp" | |
| `@echo` " Check the version with: {{ build_dir }}/testapp version" | |
| `@echo` " Check the binary with: {{ build_dir }}/testapp" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.just/build.just around lines 8 - 9, The echo messages are inconsistent: one
hardcodes "build/testapp" while the other uses the template variable "{{
build_dir }}/testapp"; update the first echo to use the same build_dir variable
so both lines reference "{{ build_dir }}/testapp" (edit the echo string in the
`@echo` that currently prints "Check the binary with: build/testapp" to use "{{
build_dir }}/testapp" instead).
| deps: | ||
| @echo "--> Installing dependencies" | ||
| @go mod download | ||
| @go mod tidy | ||
| @go run scripts/tidy.go |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the full codegen.just file to see both recipes
cat -n .just/codegen.justRepository: evstack/ev-node
Length of output: 578
🏁 Script executed:
# Check for build constraints in scripts/tidy.go
head -30 scripts/tidy.goRepository: evstack/ev-node
Length of output: 704
Add -tags=tidy flag to deps recipe on line 13.
The scripts/tidy.go file has a //go:build tidy build constraint (line 1), which requires the -tags=tidy flag to compile. Line 13 runs the script without this flag, causing it to fail. Align with the tidy-all recipe on line 18, which correctly includes -tags=tidy.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.just/codegen.just around lines 9 - 13, The deps recipe currently runs "go
run scripts/tidy.go" without the build tag, which fails because scripts/tidy.go
has a //go:build tidy constraint; update the deps recipe to invoke the script
with the build tag (use "go run -tags=tidy scripts/tidy.go") so it compiles like
the tidy-all recipe and includes the required -tags=tidy flag.
| @rm -rf client/crates/types/src/proto/*.rs | ||
| @cd client/crates/types && cargo build |
There was a problem hiding this comment.
Missing EV_TYPES_FORCE_PROTO_GEN=1 in rust-proto-check will cause false failures.
The rust-proto-check recipe deletes the generated proto files and then runs cargo build without EV_TYPES_FORCE_PROTO_GEN=1. Since rust-proto-gen uses this env var to trigger regeneration, omitting it here means the proto files won't be regenerated during the check, causing git diff to always detect changes.
🐛 Proposed fix
`@rm` -rf client/crates/types/src/proto/*.rs
- `@cd` client/crates/types && cargo build
+ `@cd` client/crates/types && EV_TYPES_FORCE_PROTO_GEN=1 cargo build📝 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.
| @rm -rf client/crates/types/src/proto/*.rs | |
| @cd client/crates/types && cargo build | |
| `@rm` -rf client/crates/types/src/proto/*.rs | |
| `@cd` client/crates/types && EV_TYPES_FORCE_PROTO_GEN=1 cargo build |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.just/proto.just around lines 24 - 25, The rust-proto-check recipe deletes
generated proto files but then runs cargo build without setting
EV_TYPES_FORCE_PROTO_GEN=1 so rust-proto-gen won’t regenerate files and git diff
will always fail; update the rust-proto-check recipe (the block that runs the
commands removing client/crates/types/src/proto/*.rs and then "cd
client/crates/types && cargo build") to run cargo build with the env var set
(e.g. prefix the build with EV_TYPES_FORCE_PROTO_GEN=1 or export it before
invoking cargo) so rust-proto-gen is triggered during the check.
| @echo "--> Running Docker E2E tests" | ||
| @echo "--> Verifying Docker image exists locally..." | ||
| @if [ -z "${EV_NODE_IMAGE_REPO:-}" ] || [ "${EV_NODE_IMAGE_REPO:-}" = "ev-node" ]; then \ | ||
| echo "--> Verifying Docker image exists locally..."; \ | ||
| docker image inspect evstack:local-dev >/dev/null 2>&1 || (echo "ERROR: evstack:local-dev image not found. Run 'just docker-build' first." && exit 1); \ | ||
| fi |
There was a problem hiding this comment.
Duplicated echo message and inconsistent repo name check.
Two issues here:
- Lines 58 and 60 duplicate the "Verifying Docker image exists locally..." message.
- Line 59 checks for
"ev-node"butdocker-build-if-localon Line 81 checks for"evstack". This inconsistency will cause unexpected behavior whenEV_NODE_IMAGE_REPO=evstack- the build recipe treats it as local, but this verification would skip it.
🐛 Proposed fix
test-docker-e2e: docker-build-if-local
`@echo` "--> Running Docker E2E tests"
- `@echo` "--> Verifying Docker image exists locally..."
- `@if` [ -z "${EV_NODE_IMAGE_REPO:-}" ] || [ "${EV_NODE_IMAGE_REPO:-}" = "ev-node" ]; then \
- echo "--> Verifying Docker image exists locally..."; \
+ `@if` [ -z "${EV_NODE_IMAGE_REPO:-}" ] || [ "${EV_NODE_IMAGE_REPO:-}" = "evstack" ]; then \
docker image inspect evstack:local-dev >/dev/null 2>&1 || (echo "ERROR: evstack:local-dev image not found. Run 'just docker-build' first." && exit 1); \
fi📝 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.
| @echo "--> Running Docker E2E tests" | |
| @echo "--> Verifying Docker image exists locally..." | |
| @if [ -z "${EV_NODE_IMAGE_REPO:-}" ] || [ "${EV_NODE_IMAGE_REPO:-}" = "ev-node" ]; then \ | |
| echo "--> Verifying Docker image exists locally..."; \ | |
| docker image inspect evstack:local-dev >/dev/null 2>&1 || (echo "ERROR: evstack:local-dev image not found. Run 'just docker-build' first." && exit 1); \ | |
| fi | |
| `@echo` "--> Running Docker E2E tests" | |
| `@if` [ -z "${EV_NODE_IMAGE_REPO:-}" ] || [ "${EV_NODE_IMAGE_REPO:-}" = "evstack" ]; then \ | |
| echo "--> Verifying Docker image exists locally..."; \ | |
| docker image inspect evstack:local-dev >/dev/null 2>&1 || (echo "ERROR: evstack:local-dev image not found. Run 'just docker-build' first." && exit 1); \ | |
| fi |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.just/test.just around lines 57 - 62, Remove the duplicated echo and make
the repo-name check consistent with the docker-build-if-local logic: delete the
repeated "@echo \"--> Verifying Docker image exists locally...\"" and change the
conditional that currently compares EV_NODE_IMAGE_REPO to "ev-node" so it
compares to "evstack" (the same value used by docker-build-if-local); keep a
single echo before the conditional and ensure the docker image inspect line
still checks evstack:local-dev.
| - name: Set up Docker Buildx | ||
| uses: docker/setup-buildx-action@b5ca514318bd6ebac0fb2aedd5d36ec1b5c232a2 # v3.10.0 | ||
| - name: Install just | ||
| uses: extractions/setup-just@v3 |
Check warning
Code scanning / CodeQL
Unpinned tag for a non-immutable Action in workflow Medium
| - name: Set up Docker Buildx | ||
| uses: docker/setup-buildx-action@b5ca514318bd6ebac0fb2aedd5d36ec1b5c232a2 # v3.10.0 | ||
| - name: Install just | ||
| uses: extractions/setup-just@v3 |
Check warning
Code scanning / CodeQL
Unpinned tag for a non-immutable Action in workflow Medium
| - name: Install make (if missing) | ||
| run: sudo apt-get update && sudo apt-get install -y make | ||
| - name: Install just | ||
| uses: extractions/setup-just@v3 |
Check warning
Code scanning / CodeQL
Unpinned tag for a non-immutable Action in workflow Medium
| with: | ||
| go-version-file: ./test/docker-e2e/go.mod | ||
| - name: Install just | ||
| uses: extractions/setup-just@v3 |
Check warning
Code scanning / CodeQL
Unpinned tag for a non-immutable Action in workflow Medium
| with: | ||
| go-version-file: ./test/docker-e2e/go.mod | ||
| - name: Install just | ||
| uses: extractions/setup-just@v3 |
Check warning
Code scanning / CodeQL
Unpinned tag for a non-immutable Action in workflow Medium
| with: | ||
| go-version-file: ./go.mod | ||
| - run: make deps | ||
| - uses: extractions/setup-just@v3 |
Check warning
Code scanning / CodeQL
Unpinned tag for a non-immutable Action in workflow Medium test
| with: | ||
| go-version-file: ./go.mod | ||
| - name: Install just | ||
| uses: extractions/setup-just@v3 |
Check warning
Code scanning / CodeQL
Unpinned tag for a non-immutable Action in workflow Medium test
| with: | ||
| go-version-file: ./go.mod | ||
| - name: Install just | ||
| uses: extractions/setup-just@v3 |
Check warning
Code scanning / CodeQL
Unpinned tag for a non-immutable Action in workflow Medium test
| cache-from: type=gha | ||
| cache-to: type=gha,mode=max | ||
| - name: Install just | ||
| uses: extractions/setup-just@v3 |
Check warning
Code scanning / CodeQL
Unpinned tag for a non-immutable Action in workflow Medium test
| with: | ||
| go-version-file: ./go.mod | ||
| - name: Install just | ||
| uses: extractions/setup-just@v3 |
Check warning
Code scanning / CodeQL
Unpinned tag for a non-immutable Action in workflow Medium test
There was a problem hiding this comment.
♻️ Duplicate comments (4)
.github/workflows/benchmark.yml (1)
27-27:⚠️ Potential issue | 🟠 MajorPin
extractions/setup-justto an immutable SHA.Line 27 and Line 59 use
extractions/setup-just@v3, which is mutable and has already been flagged by CodeQL. Please pin to a full commit hash.🔒 Proposed fix
- - name: Install just - uses: extractions/setup-just@v3 + - name: Install just + uses: extractions/setup-just@<full-commit-sha> # v3 ... - - name: Install just - uses: extractions/setup-just@v3 + - name: Install just + uses: extractions/setup-just@<full-commit-sha> # v3Also applies to: 59-59
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/benchmark.yml at line 27, Replace the mutable action reference "extractions/setup-just@v3" with an immutable pinned commit SHA for both occurrences; find the usages of extractions/setup-just@v3 in the workflow and change them to extractions/setup-just@<full-commit-sha> (use the authoritative commit hash from the action’s repository) so the workflow is pinned to a specific immutable version..github/workflows/docker-tests.yml (1)
31-31:⚠️ Potential issue | 🟠 MajorPin
extractions/setup-justin all three jobs.Line 31, Line 50, and Line 69 use mutable
@v3tags. Please pin each usage to an immutable commit SHA.🔒 Proposed fix
- - name: Install just - uses: extractions/setup-just@v3 + - name: Install just + uses: extractions/setup-just@<full-commit-sha> # v3Also applies to: 50-50, 69-69
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/docker-tests.yml at line 31, Replace the mutable tag "extractions/setup-just@v3" used in the workflow jobs with an immutable commit SHA for each usage; locate the three occurrences of the uses string (currently "extractions/setup-just@v3") and change them to "extractions/setup-just@<full-commit-sha>" (the full 40-character SHA from the setup-just repository) so the workflow is pinned to a specific commit. Ensure each occurrence is updated (use the same pinned SHA if you want identical behavior across jobs) and run a quick workflow lint or dry-run to verify the action resolves correctly..github/workflows/dependabot-auto-fix.yml (1)
26-26:⚠️ Potential issue | 🟠 MajorUse an immutable commit ref for
setup-just.Line 26 uses
extractions/setup-just@v3(mutable tag). Pinning to a commit SHA is recommended to avoid action supply-chain drift.🔒 Proposed fix
- - name: Install just - uses: extractions/setup-just@v3 + - name: Install just + uses: extractions/setup-just@<full-commit-sha> # v3🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/dependabot-auto-fix.yml at line 26, The workflow currently references the mutable tag "extractions/setup-just@v3"; replace that mutable tag with an immutable commit SHA to prevent supply-chain drift. Locate the uses line referencing "extractions/setup-just@v3" in the dependabot-auto-fix.yml and update it to the action's pinned ref (extractions/setup-just@<commit-sha>) by finding the latest commit SHA from the action's repository and substituting it for the tag..github/workflows/test.yml (1)
21-21:⚠️ Potential issue | 🟠 MajorPin
setup-justreferences to commit SHAs.Line 21, Line 33, Line 51, Line 70, Line 104, and Line 119 use
extractions/setup-just@v3(mutable tag). Pinning to immutable commit hashes is safer.🔒 Proposed fix
- - name: Install just - uses: extractions/setup-just@v3 + - name: Install just + uses: extractions/setup-just@<full-commit-sha> # v3Also applies to: 33-33, 51-51, 70-70, 104-104, 119-119
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/test.yml at line 21, Replace mutable tag references of the action "extractions/setup-just@v3" with immutable commit SHAs: locate every occurrence of the string extractions/setup-just@v3 in the workflow and replace each with extractions/setup-just@<commit-sha> using the specific commit hash from the action's repository (one unique SHA per occurrence or the same pinned SHA if you want a single fixed version across the file). Ensure you update all occurrences noted (the instances that currently read extractions/setup-just@v3) and commit the workflow changes so the runs use the pinned immutable SHAs.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In @.github/workflows/benchmark.yml:
- Line 27: Replace the mutable action reference "extractions/setup-just@v3" with
an immutable pinned commit SHA for both occurrences; find the usages of
extractions/setup-just@v3 in the workflow and change them to
extractions/setup-just@<full-commit-sha> (use the authoritative commit hash from
the action’s repository) so the workflow is pinned to a specific immutable
version.
In @.github/workflows/dependabot-auto-fix.yml:
- Line 26: The workflow currently references the mutable tag
"extractions/setup-just@v3"; replace that mutable tag with an immutable commit
SHA to prevent supply-chain drift. Locate the uses line referencing
"extractions/setup-just@v3" in the dependabot-auto-fix.yml and update it to the
action's pinned ref (extractions/setup-just@<commit-sha>) by finding the latest
commit SHA from the action's repository and substituting it for the tag.
In @.github/workflows/docker-tests.yml:
- Line 31: Replace the mutable tag "extractions/setup-just@v3" used in the
workflow jobs with an immutable commit SHA for each usage; locate the three
occurrences of the uses string (currently "extractions/setup-just@v3") and
change them to "extractions/setup-just@<full-commit-sha>" (the full 40-character
SHA from the setup-just repository) so the workflow is pinned to a specific
commit. Ensure each occurrence is updated (use the same pinned SHA if you want
identical behavior across jobs) and run a quick workflow lint or dry-run to
verify the action resolves correctly.
In @.github/workflows/test.yml:
- Line 21: Replace mutable tag references of the action
"extractions/setup-just@v3" with immutable commit SHAs: locate every occurrence
of the string extractions/setup-just@v3 in the workflow and replace each with
extractions/setup-just@<commit-sha> using the specific commit hash from the
action's repository (one unique SHA per occurrence or the same pinned SHA if you
want a single fixed version across the file). Ensure you update all occurrences
noted (the instances that currently read extractions/setup-just@v3) and commit
the workflow changes so the runs use the pinned immutable SHAs.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/benchmark.yml.github/workflows/dependabot-auto-fix.yml.github/workflows/docker-tests.yml.github/workflows/test.yml
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3110 +/- ##
==========================================
- Coverage 60.95% 60.91% -0.04%
==========================================
Files 113 113
Lines 11617 11617
==========================================
- Hits 7081 7077 -4
- Misses 3738 3740 +2
- Partials 798 800 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Inline go build/install commands directly instead of depending on make/just inside the container. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
⚠️ Performance Alert ⚠️
Possible performance regression was detected for benchmark 'Spamoor Trace Benchmarks'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.50.
| Benchmark suite | Current: da1f427 | Previous: 212ac08 | Ratio |
|---|---|---|---|
SpamoorSmoke - Executor.GetTxs (avg) |
1515.6944444444443 us |
915 us |
1.66 |
SpamoorSmoke - TxPool.GetTxs (avg) |
1498.8333333333333 us |
899.5588235294117 us |
1.67 |
This comment was automatically generated by workflow using github-action-benchmark.
|
3 participants
Replace Makefile + 6 .mk include files with a single justfile. Update all CI workflows (setup-just action) and docs references.
Overview
Summary by CodeRabbit
Chores
Documentation