base / base/contracts

just check and just pre-pr fail: scripts/checks/reinitializer does not exist

Open
#410 2 comments 0 reactions 0 assignees View on GitHub
Dominant language
Solidity
Stars
325
Forks
245
Avg merge
13h 48m
Merged PRs (30d)
18

Description

`just check` runs `reinitializer-check-no-build`, which shells out to a package that isn't in the tree:

```make
# justfile:245
reinitializer-check-no-build:
go run ./scripts/checks/reinitializer
```

`scripts/checks/` contains `common`, `interfaces`, `spacers` and `test-validation` — there is no `reinitializer`. The sibling targets use the same relative form (`go run ./scripts/checks/interfaces`) and those packages do exist, so this one looks left behind rather than pathed differently.

## Why it matters

`check` invokes the sub-checks as one `just` command, so the failure stops the chain:

```make
check:
@just semgrep-test-validity-check \
semgrep \
lint-check \
snapshots-check-no-build \
unused-imports-check-no-build \
validate-deploy-configs \
validate-spacers-no-build \
reinitializer-check-no-build \ # <- fails here
interfaces-check-no-build \ # <- never runs
lint-forge-tests-check-no-build # <- never runs
```

So two checks that do exist never get a chance to run.

It also breaks `just pre-pr` (and `just pre-commit`, which aliases it). That recipe is `set -e` and calls `just check` partway through:

```bash
just lint
just build-source
just check # <- aborts here

# Restore build artifacts after running checks.
if [ -d "$TEMP_BUILD_DIR" ]; then
...
```

The artifact-restore block after it is skipped too, so the dev-build cache the recipe went to some trouble to save isn't put back.

## Why it isn't showing up in CI

CI doesn't call `check`, `pre-pr` or `pre-commit` — the workflows use `just deps`, `just forge-build`, `just lint-check`, `just semver-lock` and `just test`. So CI stays green while the local "run all checks" path is broken.

## On the check itself

Both targets describe it as "Checks that all upgrade/initialize functions have proper reinitializer modifiers". Grepping the tree, nothing else implements that: the only `reinitializer` hits are OpenZeppelin's modifier in `src/vendor/Initializable.sol` and a comment in `test/universal/OptimismMintableERC20Factory.t.sol`. So the check the justfile advertises isn't running anywhere right now.

## Fix

I haven't sent a PR because the right fix depends on something I can't tell from outside: whether the script was dropped deliberately (in which case `reinitializer-check`, `reinitializer-check-no-build` and the line in `check` should go) or whether it went missing and should come back. Happy to send whichever you'd prefer.

Verified against `main` at `4f7acda`, and `scripts/checks/` on GitHub shows the same four packages.

Contributor guide

No contributing guide indexed for this repository

Research direction

Read the reinitializer-check-no-build and reinitializer-check targets around justfile:245, then inspect scripts/checks/ alongside the pre-pr and check recipes. Confirm with the maintainer whether the missing check should be removed or restored; done means the intended local check chain runs through its remaining checks and pre-pr completes its artifact-restore path.

Written by the indexing model from the issue text.

Assessment

Tech stack
go, shell, solidity
Domain
build-system, developer-experience, tooling
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.