Add reproducible Foundry baseline#4
Conversation
|
@claude please review this Gate A reproducible-baseline PR for contributor setup, CI/tooling portability, and whether the compile-surface randomizer import fixes are appropriate for making |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughAdds repository formatting and EOL rules, Foundry config and Make targets, cross-platform bootstrap and check scripts, CI workflow, smart-contract interface retyping to IStream*, and documentation/ops updates to establish a reproducible Gate A baseline. ChangesGate A Reproducible Baseline
Sequence Diagram(s)sequenceDiagram
participant Developer
participant BootstrapScript
participant FoundryRelease
participant PythonTooling
participant CheckScript
Developer->>BootstrapScript: run scripts/bootstrap-ec2.sh or scripts/bootstrap-windows.ps1
BootstrapScript->>FoundryRelease: download archive + .sha256 and verify checksum
BootstrapScript->>FoundryRelease: extract to install dir (~/.foundry/bin or $InstallDir)
BootstrapScript->>PythonTooling: create .venv-tools, install requirements-tools.txt, select solc 0.8.19
Developer->>CheckScript: run scripts/check.sh or scripts/check.ps1
CheckScript->>FoundryRelease: run forge build then forge test -vvv
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?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: 5
🧹 Nitpick comments (1)
scripts/check.sh (1)
7-10: ⚡ Quick winAdd an explicit
forgepreflight check for clearer Linux setup failures.Right now, missing
forgefails with a generic command-not-found path. Mirror the Windows script’s explicit check so contributors get actionable setup guidance.Proposed patch
export PATH="$HOME/.foundry/bin:$PATH" +if ! command -v forge >/dev/null 2>&1; then + echo "forge was not found. Run scripts/bootstrap-ec2.sh, then retry this command." >&2 + exit 1 +fi + forge build forge test -vvv🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/check.sh` around lines 7 - 10, Add an explicit preflight check for the `forge` binary before running `forge build`/`forge test`: use `command -v forge >/dev/null 2>&1` (or `which forge`) to detect absence, print a clear actionable error message instructing how to install Foundry (matching the Windows script guidance), and exit with a non-zero status if missing; place this check immediately before the existing `forge build` invocation so `forge build` and `forge test -vvv` only run when `forge` is available.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 15-16: Update the Checkout step that uses actions/checkout@v4 to
explicitly set persist-credentials: false so the runner does not persist the
GITHUB_TOKEN in the workspace; locate the step referencing uses:
actions/checkout@v4 and add the persist-credentials: false input to that step to
reduce token exposure for later steps.
- Line 16: Replace mutable action tags with immutable 40-character commit SHAs
for every `uses:` entry (e.g., change `actions/checkout@v4` to
`actions/checkout@<commit-sha>`) so CI is not vulnerable to upstream tag
retargeting; update all occurrences (including the other `uses:` entries
flagged) and ensure each `uses:` value is a full 40-char SHA rather than `@vX`
or other mutable tags.
In `@ops/AUTONOMOUS_RUN.md`:
- Line 62: Update the markdown header "### PR 2: Reproducible baseline tooling"
to match the correct GitHub PR number by changing it to "### PR `#4`: Reproducible
baseline tooling" (or alternatively "### Queue Item 2: Reproducible baseline
tooling" if you intend to reference queue order); search for the exact header
string "PR 2: Reproducible baseline tooling" and replace it so the worklog
header matches the identifier used later on (line referencing Pull Request `#4`).
In `@scripts/bootstrap-ec2.sh`:
- Line 29: The script currently calls foundryup with the -v flag which only
prints the tool version instead of installing/switching to the requested
release; update the invocation in scripts/bootstrap-ec2.sh to call the install
subcommand with the pinned version (e.g., use foundryup install
"$FOUNDRY_VERSION" or the foundryup command form that explicitly
installs/switches to the provided version) so the environment actually uses the
FOUNDRY_VERSION variable, and then validate by running forge --version to ensure
the pinned release is active.
In `@scripts/bootstrap-windows.ps1`:
- Line 57: The script currently calls "python -m venv .venv-tools" directly
which fails if python isn't on PATH; change the venv creation to detect and
prefer the "python" executable but fall back to the Windows Python launcher "py"
if "python" isn't found, and fail with a clear message if neither is available.
Implement detection using PowerShell command existence checks (e.g.,
Get-Command) and then invoke either "python -m venv .venv-tools" or "py -m venv
.venv-tools" accordingly, ensuring the chosen launcher is stored/used
consistently for subsequent Python operations in the script. Ensure error
handling logs which launcher was attempted when reporting failure.
---
Nitpick comments:
In `@scripts/check.sh`:
- Around line 7-10: Add an explicit preflight check for the `forge` binary
before running `forge build`/`forge test`: use `command -v forge >/dev/null
2>&1` (or `which forge`) to detect absence, print a clear actionable error
message instructing how to install Foundry (matching the Windows script
guidance), and exit with a non-zero status if missing; place this check
immediately before the existing `forge build` invocation so `forge build` and
`forge test -vvv` only run when `forge` is available.
🪄 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: b68491db-f711-4f2f-9c31-e9d552725ec7
📒 Files selected for processing (21)
.editorconfig.gitattributes.github/workflows/ci.yml.gitignoreMakefileREADME.mddocs/adr/README.mddocs/known-blockers.mddocs/status.mddocs/tooling.mdfoundry.tomlops/AUTONOMOUS_RUN.mdrequirements-tools.txtscript/README.mdscripts/bootstrap-ec2.shscripts/bootstrap-windows.ps1scripts/check.ps1scripts/check.shsmart-contracts/RandomizerRNG.solsmart-contracts/RandomizerVRF.soltest/README.md
|
@claude pushed
Validation rerun locally: |
|
@claude pushed
Please re-review this PR. |
| if (-not $SkipPathUpdate) { | ||
| $userPath = [Environment]::GetEnvironmentVariable("Path", "User") | ||
| $pathParts = @() | ||
| if ($userPath) { | ||
| $pathParts = $userPath -split ";" | Where-Object { $_ -ne "" } | ||
| } | ||
|
|
||
| $normalizedInstallDir = $InstallDir.TrimEnd("\") | ||
| $alreadyPresent = $pathParts | Where-Object { $_.TrimEnd("\") -ieq $normalizedInstallDir } | ||
| if (-not $alreadyPresent) { | ||
| [Environment]::SetEnvironmentVariable("Path", (($pathParts + $InstallDir) -join ";"), "User") | ||
| Write-Host "Added $InstallDir to the user PATH. Open a new terminal to use it automatically." | ||
| } | ||
| } |
There was a problem hiding this comment.
🔴 🔴 scripts/bootstrap-windows.ps1 lines 75-88 silently corrupt the persistent User PATH when it has exactly one non-empty entry. PowerShell pipeline assignment auto-unrolls a single-element result on line 79, so $pathParts becomes a scalar [string] (the $pathParts = @() on line 77 is overwritten). Line 85's ($pathParts + $InstallDir) is then string concatenation (no separator), and SetEnvironmentVariable durably writes a mashed entry like …\WindowsAppsC:\Users\foo\.foundry\bin to HKCU PATH — the script prints "Bootstrap complete" with no error, but the next shell has both WindowsApps and .foundry\bin unresolvable. The single-entry case is the default state on fresh Windows 10/11 (only %USERPROFILE%\AppData\Local\Microsoft\WindowsApps in User PATH) — exactly the persona this onboarding script targets. Fix: wrap with @(...) to force array semantics — $pathParts = @($userPath -split ";" | Where-Object { $_ -ne "" }).
Extended reasoning...
What the bug is
scripts/bootstrap-windows.ps1 lines 75-88 update the persistent User PATH via this idiom:
$userPath = [Environment]::GetEnvironmentVariable("Path", "User")
$pathParts = @()
if ($userPath) {
$pathParts = $userPath -split ";" | Where-Object { $_ -ne "" }
}
$normalizedInstallDir = $InstallDir.TrimEnd("\")
$alreadyPresent = $pathParts | Where-Object { $_.TrimEnd("\") -ieq $normalizedInstallDir }
if (-not $alreadyPresent) {
[Environment]::SetEnvironmentVariable("Path", (($pathParts + $InstallDir) -join ";"), "User")
Write-Host "Added $InstallDir to the user PATH. Open a new terminal to use it automatically."
}The line 77 $pathParts = @() initialization is overwritten on line 79 by the pipeline result. In PowerShell, pipeline output assigned to a variable is auto-unrolled when it emits exactly one item — $pathParts becomes a scalar [string], not a single-element array. That's the canonical PowerShell footgun this bug is about.
Step-by-step proof (default Windows 10/11, fresh contributor)
A default Windows 10/11 User PATH typically contains exactly one entry: %USERPROFILE%\AppData\Local\Microsoft\WindowsApps, added automatically by Windows for the App Execution Alias system. So:
- Line 76:
$userPath = "C:\Users\foo\AppData\Local\Microsoft\WindowsApps"(single non-empty entry). - Line 77:
$pathParts = @()(empty array — immediately overwritten by line 79). - Line 79:
$userPath -split ";"returns@("C:\…\WindowsApps")(1-element array);| Where-Object { $_ -ne "" }emits the single non-empty string; pipeline assignment unrolls it to a scalar[string].$pathPartsis now a[string], not an array. - Line 83:
$pathParts | Where-Object { … }— piping a scalar through Where-Object works (PowerShell pipes scalars), but it doesn't match$normalizedInstallDir, so$alreadyPresentis$null. - Line 85:
($pathParts + $InstallDir)— when the left operand is a[string],+is string concatenation, not array append. Result:"C:\Users\foo\AppData\Local\Microsoft\WindowsAppsC:\Users\foo\.foundry\bin"(no separator). -join ";"on a single scalar string is a no-op.SetEnvironmentVariable("Path", $mashed, "User")durably writes the corrupted single-entry value to HKCU PATH.- Script prints
"Added $InstallDir to the user PATH..."and exits withBootstrap complete.
Why existing code doesn't prevent it
The $pathParts = @() on line 77 looks defensive but is overwritten on line 79 by the unrolled pipeline result. There is no @(...) wrap around the pipeline to force array semantics. The empty-PATH case is safe (line 77's @() is kept), and the 2+-entry case is safe (the pipeline emits multiple items, so the assignment captures an Object[]). Only the exactly-1-entry case triggers — but that's the default state on the fresh-Windows persona this script explicitly targets.
The PR's Windows validation only ran scripts\check.ps1 (per ops/AUTONOMOUS_RUN.md), which does not exercise the bootstrap PATH-rewrite path. The Claude and CodeRabbit reviews flagged -UseBasicParsing, the Python launcher fallback, and CI-action pinning, but did not catch this distinct array-unrolling bug. The recent commit 47a23d1 added -UseBasicParsing and Python preflight, but lines 75-88 are unchanged.
Impact
Silent, durable corruption of HKCU PATH — the worst kind of side effect for a contributor-onboarding script. After bootstrap "succeeds":
- WindowsApps becomes unresolvable on the next shell (
python,winget, etc. break via the App Execution Alias system). .foundry\bin\forge.exeis also not on PATH (glued to WindowsApps without a separator).- No error signal — the script prints
Bootstrap complete. - Recovery requires opening the Environment Variables UI and pasting back a properly-separated PATH.
Fix
The canonical one-character idiom — wrap the pipeline in @(...) to force array semantics regardless of element count:
$pathParts = @($userPath -split ";" | Where-Object { $_ -ne "" })With @(...), the result is always an array (0, 1, or N items), so:
$pathParts + $InstallDiris array+scalar = array append.-join ";"produces a correctly semicolon-separated PATH.
This also makes the $pathParts = @() initializer on line 77 redundant (the @(...) wrap covers the empty-string case too), though leaving it in place is harmless.
| slither-analyzer==0.11.5 | ||
| solc-select==1.2.0 |
There was a problem hiding this comment.
🔴 The pinned slither-analyzer==0.11.5 in requirements-tools.txt requires Python >=3.10 per PyPI metadata, but README.md:40, docs/tooling.md:52, and the bootstrap-windows.ps1:57 error message all say "Python 3.8+". Neither bootstrap-ec2.sh (which installs the distro default python3 — 3.7 on AL2, 3.8 on Ubuntu 20.04, 3.9 on AL2023/Debian 11/RHEL 9) nor bootstrap-windows.ps1 (whose Resolve-Python only filters out WindowsApps stubs) validates the interpreter version before pip install. A fresh contributor on any of those common images runs the bootstrap, hits a cryptic Package slither-analyzer requires a different Python: 3.x not in >=3.10, and set -euo pipefail aborts the whole bootstrap — directly contradicting the docs they just followed. Fix: either pin a 3.8-compatible slither line, or raise the documented floor to "Python 3.10+" uniformly in README.md / docs/tooling.md / the PS1 error message AND add an explicit sys.version_info >= (3, 10) preflight in both bootstrap scripts before pip install.
Extended reasoning...
What the bug is
requirements-tools.txt pins slither-analyzer==0.11.5, whose PyPI metadata explicitly declares Requires-Python: ">=3.10" (and classifiers list only Python 3.10–3.14). Both bootstrap scripts add this requirements file to a freshly-created .venv-tools without first checking the interpreter version, while the documentation and the Windows error message advertise Python 3.8+ as sufficient.
Specifically:
requirements-tools.txt:1—slither-analyzer==0.11.5(Requires-Python: >=3.10)README.md:40— "On Windows, install Python 3.8+ or thepylauncher"docs/tooling.md:52— "Windows bootstrap requires Python 3.8+ or thepylauncher"scripts/bootstrap-windows.ps1:57—throw "Python 3.8+ is required. Install Python from python.org or install the py launcher, then re-run this script."scripts/bootstrap-ec2.sh:9–17—install_packages()just runsapt-get/dnf/yum install -y ... python3 ...with no version pinscripts/bootstrap-ec2.sh:79—python3 -m venv .venv-toolsthenpip install -r requirements-tools.txt, no preflightscripts/bootstrap-windows.ps1:36–58—Resolve-Pythononly filters outWindowsAppsreparse stubs; thepy -3fallback accepts any Python 3.x including 3.7/3.8/3.9
Why the existing code does not prevent it
The Linux path trusts whatever python3 the distro packages. The Windows path trusts whatever python or py -3 resolves to that is not a WindowsApps stub. set -euo pipefail aborts on the pip failure but only after producing a generic pip error — the script never says "your Python is too old"; the docs the user just followed said 3.8 was fine. solc-select==1.2.0 (the other pinned tool) declares Requires-Python: ">=3.8", so it does not force the floor up — only slither does.
Step-by-step proof
- Provision a fresh Amazon Linux 2 EC2 instance (
python3→ 3.7) — exactly the personabootstrap-ec2.shis named for. - Clone the repo and run
bash scripts/bootstrap-ec2.sh. install_packages()installspython3(3.7);install_foundry()succeeds.install_python_tools()runspython3 -m venv .venv-tools— succeeds with the 3.7 interpreter.pip install -r requirements-tools.txtfails:ERROR: Package slither-analyzer requires a different Python: 3.7.x not in >=3.10.set -euo pipefailexits the bootstrap non-zero, leaving the contributor with Foundry installed but no slither/solc-select, and an error that does not reference the documented "Python 3.8+" claim.
Same outcome on Ubuntu 20.04 (3.8), Amazon Linux 2023 / Debian 11 / RHEL 9 (3.9). On Windows, a contributor who followed README.md:40 and installed Python 3.8 or 3.9 from python.org hits the same pip failure — Resolve-Python returns that interpreter happily.
Impact
This is the Gate A contributor-onboarding promise of this PR, and the bootstrap network/venv path was not validated end-to-end on a fresh image (the PR description shows validation was forge build / forge test / make check on an already-configured machine). The most common EC2 AMI families (AL2, AL2023) and Ubuntu 20.04 / Debian 11 / RHEL 9 all fail. The user-visible symptom is a cryptic pip error against documentation that promised 3.8+ would suffice.
How to fix
Pick one (the verifiers consistently propose the same two options):
- Pin a Python-3.8-compatible slither line (e.g. an older
0.10.xrelease that supports Python 3.8+). Keeps the docs accurate. - Raise the documented floor to "Python 3.10+" uniformly in
README.md:40,docs/tooling.md:52, and thebootstrap-windows.ps1:57error message, AND add a preflight in both bootstrap scripts before thepip install, for example:- Linux:
python3 -c "import sys; sys.exit(0 if sys.version_info >= (3, 10) else 1)" || { echo "Python 3.10+ required (found $(python3 -V))"; exit 1; } - Windows: a matching
$ver = & $python.FilePath -c "import sys; print(f{sys.version_info.major}.{sys.version_info.minor})"check that throws a clear actionable error.
- Linux:
Either way, align the docs with the actual floor that requirements-tools.txt imposes, so the bootstrap can fail fast and informatively instead of dying inside pip.
| [*] | ||
| charset = utf-8 | ||
| end_of_line = lf | ||
| insert_final_newline = true | ||
| indent_style = space | ||
| indent_size = 4 | ||
| trim_trailing_whitespace = true |
There was a problem hiding this comment.
🟡 The new .editorconfig sets indent_size = 4 under [*] with no overrides for YAML or shell, but the YAML and shell files added in this same PR — .github/workflows/ci.yml, scripts/bootstrap-ec2.sh, scripts/check.sh — all use 2-space indentation. Editors that honor .editorconfig (VS Code with the EditorConfig extension, JetBrains, vim-editorconfig) will auto-indent newly typed lines at 4 spaces in these files, producing mixed-indent diffs and — for YAML, where indentation is structurally significant — potentially breaking the document. One-line fix: add [*.{yml,yaml}] and [*.sh] blocks with indent_size = 2 next to the existing [Makefile] / [*.md] / [*.ps1] overrides.
Extended reasoning...
What the bug is. .editorconfig lines 3-9 declare [*] with indent_style = space and indent_size = 4. The only overrides are [Makefile] (tab), [*.md] (whitespace), and [*.ps1] (CRLF) — none of which set indent_size for YAML or shell. Yet the files this PR introduces under those globs use 2-space indentation: .github/workflows/ci.yml is standard 2-space YAML, and scripts/bootstrap-ec2.sh / scripts/check.sh use 2-space throughout.
Why existing config doesn't prevent it. There is no glob entry for *.yml, *.yaml, or *.sh, so they all fall through to the [*] default of 4. The .editorconfig and the committed files therefore disagree about indent width for the very files the PR adds.
Step-by-step proof. A contributor opens .github/workflows/ci.yml in VS Code with the EditorConfig extension (or JetBrains IDE / vim-editorconfig). They hit Enter inside the existing jobs.foundry.steps block, currently indented at 6 spaces (3 levels × 2 spaces). The editor reads .editorconfig, sees indent_size = 4 applies to this file, and inserts 4 spaces for the next level instead of 2. The new line is now at 10 spaces instead of 8, which is invalid YAML structurally — YAML treats different indentation widths within the same block as a parse error or as a different scope. For scripts/check.sh and scripts/bootstrap-ec2.sh the consequence is less severe (bash doesn't care about indent width) but the file accumulates mixed 2/4-space blocks that produce noisy diffs.
Impact. Low — .editorconfig is editor-side guidance, not a CI gate, and the committed files themselves are well-formed today. The breakage is contributor-side and only manifests when a contributor edits one of these files in an EditorConfig-aware editor without manually overriding indent. However, the asymmetry is real: the PR explicitly establishes a reproducible-baseline formatting layer and goes to the trouble of overriding [Makefile], [*.md], and [*.ps1] — leaving YAML and shell out is an obvious gap in the same set of files.
How to fix. One line per filetype:
[*.{yml,yaml}]
indent_size = 2
[*.sh]
indent_size = 2Alternatively, invert the default to indent_size = 2 and add [*.sol] indent_size = 4 for Solidity (which is 4-space in this repo). Either approach restores agreement between .editorconfig and the source-of-truth indentation used in the files the PR ships.
| forge build | ||
| forge test -vvv |
There was a problem hiding this comment.
🟡 scripts/check.ps1 runs forge build and forge test -vvv sequentially (lines 19-20) without checking $LASTEXITCODE between them. In PowerShell 5.1 (the declared target via #Requires -Version 5.1), $ErrorActionPreference = 'Stop' only governs cmdlet errors — native command non-zero exits set $LASTEXITCODE but do not terminate the script. So if forge build fails, forge test -vvv still runs, and the final exit code reflects only forge test. With the documented "no tests found" state where forge test exits 0, a forge build failure can be silently masked. This is asymmetric with scripts/check.sh's set -euo pipefail and with the sibling scripts/bootstrap-windows.ps1:23-34 Invoke-Native helper added in the same PR. Fix: reuse the Invoke-Native helper or add if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE } after each forge invocation.
Extended reasoning...
What the bug is
scripts/check.ps1 lines 19-20 invoke two native commands in sequence:
forge build
forge test -vvvThe script declares #Requires -Version 5.1 on line 1, so Windows PowerShell 5.1 is the supported target. In PS 5.1, $ErrorActionPreference = 'Stop' (line 5) governs cmdlet errors only. Native executables like forge.exe set $LASTEXITCODE on failure but do not raise a terminating error — they continue execution. The opt-in fix ($PSNativeCommandUseErrorActionPreference) only landed in PowerShell 7.3+, so PS 5.1 users get no automatic propagation.
Result: a failed forge build does not stop the script. forge test -vvv runs anyway, and the script's final exit code becomes whatever forge test returned.
Why the existing code doesn't prevent it
$ErrorActionPreference = 'Stop' is the only error-handling layer in check.ps1. There is no Invoke-Native wrapper, no manual $LASTEXITCODE check, no -ErrorAction plumbing. Crucially, this same PR's sibling script scripts/bootstrap-windows.ps1:23-34 already implements exactly the right pattern:
function Invoke-Native {
param([Parameter(Mandatory = $true)][string]$FilePath, [string[]]$Arguments = @())
& $FilePath @Arguments
if ($LASTEXITCODE -ne 0) {
throw "$FilePath failed with exit code $LASTEXITCODE."
}
}That helper was added in commit 47a23d1 in direct response to Claude's earlier review flagging this exact PS 5.1 footgun. The same fix was not propagated to check.ps1, even though check.ps1 is the documented Windows smoke entrypoint referenced from README.md and docs/tooling.md.
Asymmetry with check.sh
The Linux peer scripts/check.sh:2 has set -euo pipefail, which aborts on the first non-zero exit. Two scripts that are presented as platform-equivalent entrypoints actually behave differently in the failure case.
Step-by-step proof
- Windows contributor runs
powershell -ExecutionPolicy Bypass -File scripts\check.ps1against a checkout where a Solidity file has a non-compilationforge buildissue (e.g. a cache write permission failure, a flag-specific build issue, or a futurescript/compile error confined to deployment scripts not included in the test compile path). - Line 19
forge buildexits non-zero.$LASTEXITCODEis set, but$ErrorActionPreference = 'Stop'does not raise a terminating error for native exit codes in PS 5.1. - Execution continues to line 20.
forge test -vvvruns. - The PR description explicitly notes "forge test -vvv currently executes but reports no tests found" — the no-tests path exits 0.
- The script exits with code 0. CI / the contributor's terminal reports success despite the build failure.
Compare with check.sh: set -euo pipefail would abort at step 2 with the build's non-zero exit. Compare with bootstrap-windows.ps1: every native call is wrapped in Invoke-Native and throws on non-zero $LASTEXITCODE.
Impact
Nit severity. The practical masking window is narrow today because forge test re-compiles sources and typically catches the same compile errors forge build would. The realistic masking case is a forge build failure that doesn't recur in forge test's code path — for example a compile error confined to script/ (Foundry's script directory is built by forge build but not by forge test), a cache/permission issue specific to a build flag, or any non-compilation forge build failure while forge test exits 0 with no tests found.
Even where the masking is unlikely to fire today, the asymmetry with the same PR's own Invoke-Native helper and with check.sh's set -euo pipefail is the substantive concern — the PR explicitly addressed this PS 5.1 footgun in one place and missed the documented Windows smoke entrypoint.
How to fix
One-line fix per invocation:
forge build
if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE }
forge test -vvv
if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE }Or, cleaner, reuse the existing helper from bootstrap-windows.ps1 (lift it into a shared module or duplicate it):
Invoke-Native -FilePath 'forge' -Arguments @('build')
Invoke-Native -FilePath 'forge' -Arguments @('test', '-vvv')1 participant
Summary
This Gate A PR makes a fresh checkout compile the actual Solidity source tree and gives contributors a reproducible smoke path.
smart-contracts/with Solidity0.8.19.make check, Windows/Linux check scripts, and bootstrap scripts for local setup.forge buildandforge test -vvv, including uploaded logs.RandomizerRNG.solandRandomizerVRF.solby importing the existing Stream interfaces instead of missing NextGen interface files.Important Limitations
forge test -vvvcurrently executes but reports no tests found. This PR intentionally documents that gap; it does not claim protocol correctness.Validation
forge buildpassed.forge test -vvvpassed command execution and reported no tests found.make checkpassed.powershell -ExecutionPolicy Bypass -File scripts\check.ps1passed.git check-ignore -v out/ cache/ broadcast/ .venv-tools/ .env.localconfirmed generated/local artifacts are ignored.Roadmap
This advances Gate A: Reproducible baseline from
ops/ROADMAP.md.Summary by CodeRabbit
Documentation
Chores
Refactor