Deterministic subgraph deploy versions#109
Conversation
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughAdded a Bash subgraph helper library and tests; refactored Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ 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 Tip CodeRabbit can use Trivy to scan for security misconfigurations and secrets in Infrastructure as Code files.Add a .trivyignore file to your project to customize which findings Trivy reports. |
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
test/bats/task/subgraph-deploy-version.test.bats (1)
17-38: Add one golden hash assertion.These tests stay green if
subgraph_deploy_version()changes its delimiter or hashes a trailing newline. One fixed input/output case would pin the algorithm down and catch that class of regression immediately.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/bats/task/subgraph-deploy-version.test.bats` around lines 17 - 38, Tests for subgraph_deploy_version are missing a golden-value assertion so changes to delimiters or trailing-newline handling can slip by; add one deterministic test that calls subgraph_deploy_version with a fixed address and commit (e.g., "0xabc" and "abc1234") and asserts the exact 8-character output equals the known canonical hash string produced by the current implementation (pinning the algorithm). Ensure the test input uses the exact bytes (no extra newline/whitespace) and add the equality assertion referencing subgraph_deploy_version so future changes that alter hashing or formatting fail the test.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@flake.nix`:
- Around line 346-347: The current script still runs "forge build" and "graph
codegen" before checking the Goldsky existence, so already-deployed networks
still perform local builds; modify the control flow so the Goldsky existence
check executes before any build steps and move both "forge build" and "graph
codegen" (and the subgraph build line "(cd ./subgraph && ${the-graph}/bin/graph
build --network \"$network\")") into the branch that runs only when the network
is not already deployed, ensuring the early-exit/skip path bypasses all build
commands.
- Line 56: The build fails because lib/subgraph.sh's subgraph_deploy_version
uses sha256sum but node-build-inputs only lists pkgs.nodejs_22 and pkgs.jq;
update the shared subgraph task inputs in flake.nix by adding GNU coreutils
(e.g., pkgs.coreutils) to node-build-inputs so sha256sum is available in the
task environment and restore subgraph-build/subgraph-deploy and bats tests on
macOS/standalone runs.
- Around line 337-340: The commit variable currently uses an abbreviated,
variable-length hash; change the assignment so commit="$( ${pkgs.git}/bin/git
rev-parse HEAD )" (i.e., use the full commit hash) so subgraph_deploy_version
receives a deterministic commit string; update the line that defines commit and
keep the rest (calls to subgraph_networks, subgraph_network_address, and
subgraph_deploy_version) unchanged.
---
Nitpick comments:
In `@test/bats/task/subgraph-deploy-version.test.bats`:
- Around line 17-38: Tests for subgraph_deploy_version are missing a
golden-value assertion so changes to delimiters or trailing-newline handling can
slip by; add one deterministic test that calls subgraph_deploy_version with a
fixed address and commit (e.g., "0xabc" and "abc1234") and asserts the exact
8-character output equals the known canonical hash string produced by the
current implementation (pinning the algorithm). Ensure the test input uses the
exact bytes (no extra newline/whitespace) and add the equality assertion
referencing subgraph_deploy_version so future changes that alter hashing or
formatting fail the test.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 147c4db4-fda4-48ed-abe1-6ca46040f74c
📒 Files selected for processing (4)
flake.nixlib/subgraph.shtest/bats/task/subgraph-deploy-version.test.batstest/fixture/.gitignore
| ]; | ||
|
|
||
| node-build-inputs = [ pkgs.nodejs_22 ]; | ||
| node-build-inputs = [ pkgs.nodejs_22 pkgs.jq ]; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "subgraph.sh" | head -20Repository: rainlanguage/rainix
Length of output: 80
🏁 Script executed:
grep -n "sha256sum" lib/subgraph.sh 2>/dev/null || echo "File not found or pattern not found"Repository: rainlanguage/rainix
Length of output: 119
🏁 Script executed:
sed -n '50,65p' flake.nixRepository: rainlanguage/rainix
Length of output: 597
🏁 Script executed:
grep -n "node-build-inputs" flake.nix | head -10Repository: rainlanguage/rainix
Length of output: 569
🏁 Script executed:
grep -n "subgraph\.sh" flake.nix | head -20Repository: rainlanguage/rainix
Length of output: 150
🏁 Script executed:
grep -n "coreutils" flake.nix | head -10Repository: rainlanguage/rainix
Length of output: 45
🏁 Script executed:
# Check if pkgs.coreutils provides sha256sum in nixpkgs
# Let's also check the full context around subgraph-deploy task
sed -n '300,360p' flake.nixRepository: rainlanguage/rainix
Length of output: 2507
🏁 Script executed:
# Check for REUSE/DCL-1.0 license headers in flake.nix
head -30 flake.nixRepository: rainlanguage/rainix
Length of output: 1327
🏁 Script executed:
# Let's check if there's a license/COPYING/etc file in the repo
find . -type f -name "*LICENSE*" -o -name "*COPYING*" -o -name "*DCL*" | head -10Repository: rainlanguage/rainix
Length of output: 96
🏁 Script executed:
# Let's also check the actual function in subgraph.sh that uses sha256sum
cat lib/subgraph.shRepository: rainlanguage/rainix
Length of output: 814
🏁 Script executed:
# Check if this is a public repo with REUSE compliance
grep -r "SPDX-License-Identifier" . --include="*.nix" | head -5Repository: rainlanguage/rainix
Length of output: 45
🏁 Script executed:
# Check what tasks include node-build-inputs and might need coreutils
grep -B 10 "additionalBuildInputs.*node-build-inputs" flake.nix | grep -E "(name|additionalBuildInputs)"Repository: rainlanguage/rainix
Length of output: 728
🏁 Script executed:
# Verify if pkgs.coreutils is a valid nixpkgs reference
# Let's check the actual task definitions that use node-build-inputs
sed -n '300,330p' flake.nix | cat -nRepository: rainlanguage/rainix
Length of output: 1290
🏁 Script executed:
# Check if there's a test file that exercises subgraph-deploy
find . -type f -name "*subgraph*test*" -o -name "*deploy*.test*" 2>/dev/null | head -10Repository: rainlanguage/rainix
Length of output: 155
🏁 Script executed:
# Let's check what's in the reuse lint checks - does flake.nix have proper license headers?
head -5 flake.nix && echo "---" && tail -5 flake.nixRepository: rainlanguage/rainix
Length of output: 233
🏁 Script executed:
# Check the test file to understand what's being tested
cat test/bats/task/subgraph-deploy-version.test.batsRepository: rainlanguage/rainix
Length of output: 1223
🏁 Script executed:
# Let's verify that pkgs.coreutils is available in nixpkgs and provides sha256sum
# Check if coreutils is mentioned anywhere else in the flake
grep -n "coreutils" flake.nixRepository: rainlanguage/rainix
Length of output: 45
🏁 Script executed:
# Check the DCL-1.0 license status by looking at REUSE config
cat .reuse/dep5 2>/dev/null || echo "No .reuse/dep5 found"Repository: rainlanguage/rainix
Length of output: 83
🏁 Script executed:
# Check if flake.nix would pass reuse lint - look for license markers
grep -n "SPDX\|REUSE\|License" flake.nix || echo "No REUSE markers found"Repository: rainlanguage/rainix
Length of output: 85
🏁 Script executed:
# Check other nix files to see if they have SPDX headers
find . -name "*.nix" -type f | head -5 | xargs head -3Repository: rainlanguage/rainix
Length of output: 112
🏁 Script executed:
# Verify what coreutils package provides in nixpkgs context
# Let's check if there are any nix files with explicit coreutils inclusion
grep -r "coreutils" . --include="*.nix" | head -10Repository: rainlanguage/rainix
Length of output: 45
🏁 Script executed:
# Check the actual flake.nix to see if it's managed under REUSE
ls -la .reuse/ 2>/dev/null || echo "No .reuse directory"Repository: rainlanguage/rainix
Length of output: 82
🏁 Script executed:
# Let's see what sol-build-inputs includes to understand the pattern
grep -A 5 "sol-build-inputs = " flake.nixRepository: rainlanguage/rainix
Length of output: 486
🏁 Script executed:
# Final check: verify that tasks using node-build-inputs actually need sha256sum
# by checking which tasks source subgraph.sh
grep -B 20 "source.*subgraph.sh" flake.nix | grep -E "name = |source.*subgraph"Repository: rainlanguage/rainix
Length of output: 247
🏁 Script executed:
# Check if there's any Darwin-specific handling already in place
grep -n "darwin\|Darwin\|macOS" flake.nix | head -10Repository: rainlanguage/rainix
Length of output: 504
Add GNU coreutils to the shared subgraph task inputs.
lib/subgraph.sh requires sha256sum for the subgraph_deploy_version function, but node-build-inputs only provides Node and jq. This breaks subgraph-build, subgraph-deploy, and the bats test coverage on macOS and in standalone task invocations where sha256sum is not available on the system PATH.
Proposed fix
- node-build-inputs = [ pkgs.nodejs_22 pkgs.jq ];
+ node-build-inputs = [ pkgs.nodejs_22 pkgs.jq pkgs.coreutils ];📝 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.
| node-build-inputs = [ pkgs.nodejs_22 pkgs.jq ]; | |
| node-build-inputs = [ pkgs.nodejs_22 pkgs.jq pkgs.coreutils ]; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@flake.nix` at line 56, The build fails because lib/subgraph.sh's
subgraph_deploy_version uses sha256sum but node-build-inputs only lists
pkgs.nodejs_22 and pkgs.jq; update the shared subgraph task inputs in flake.nix
by adding GNU coreutils (e.g., pkgs.coreutils) to node-build-inputs so sha256sum
is available in the task environment and restore subgraph-build/subgraph-deploy
and bats tests on macOS/standalone runs.
| commit="$(${pkgs.git}/bin/git rev-parse --short HEAD)" | ||
| for network in $(subgraph_networks ./subgraph/networks.json); do | ||
| address=$(subgraph_network_address ./subgraph/networks.json "$network") | ||
| version=$(subgraph_deploy_version "$address" "$commit") |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's confirm the file and lines in question
head -n 345 flake.nix | tail -n 15Repository: rainlanguage/rainix
Length of output: 902
🏁 Script executed:
# Also check if there are any license headers in the file
head -n 50 flake.nixRepository: rainlanguage/rainix
Length of output: 1874
🏁 Script executed:
# Get broader context around the lines mentioned (337-340)
sed -n '330,350p' flake.nixRepository: rainlanguage/rainix
Length of output: 1265
🏁 Script executed:
# Check for REUSE license headers and compliance
head -n 10 flake.nix | cat -ARepository: rainlanguage/rainix
Length of output: 462
🏁 Script executed:
# Search for other git rev-parse usages in the file
rg "git rev-parse" flake.nix -nRepository: rainlanguage/rainix
Length of output: 133
🏁 Script executed:
# Check if this is a REUSE compliant repository
if [ -f LICENSES/DCL-1.0.txt ] || [ -f LICENSES/DCL-1.0.md ]; then
echo "DCL-1.0 license found"
ls -la LICENSES/ 2>/dev/null | head -20
fiRepository: rainlanguage/rainix
Length of output: 45
🏁 Script executed:
# Check reuse compliance status
cd . && find . -name "*.license" -o -name "LICENSES" -type dRepository: rainlanguage/rainix
Length of output: 73
🏁 Script executed:
# Check the LICENSES directory
ls -la LICENSES/Repository: rainlanguage/rainix
Length of output: 249
🏁 Script executed:
# Check if flake.nix has SPDX license headers
head -c 500 flake.nix | grep -i "SPDX\|License"Repository: rainlanguage/rainix
Length of output: 45
🏁 Script executed:
# Look at the entire flake.nix file structure to find license info
wc -l flake.nixRepository: rainlanguage/rainix
Length of output: 76
🏁 Script executed:
# Check for any license comments at the top or bottom of flake.nix
tail -20 flake.nixRepository: rainlanguage/rainix
Length of output: 1320
🏁 Script executed:
# Check if there's a .reuse directory
ls -la .reuse 2>/dev/null || echo "No .reuse directory"Repository: rainlanguage/rainix
Length of output: 82
🏁 Script executed:
# Check if flake.nix has REUSE compliant license header
# REUSE requires either SPDX headers or .license files
ls flake.nix.license 2>/dev/null || echo "No .license file"Repository: rainlanguage/rainix
Length of output: 79
🏁 Script executed:
# Look at the actual license filename - check if it should be DCL-1.0.txt
cat LICENSES/LicenseRef-DCL-1.0.txt | head -20Repository: rainlanguage/rainix
Length of output: 1135
🏁 Script executed:
# Check the exact REUSE requirements for this repository
find . -name "README*" -o -name "COPYING*" | head -5Repository: rainlanguage/rainix
Length of output: 99
🏁 Script executed:
# Search for how other .nix files handle licensing in this repo
find . -name "*.nix" -type f | head -10 | xargs head -5Repository: rainlanguage/rainix
Length of output: 167
🏁 Script executed:
# Check if reuse tool is available and run reuse lint
if command -v reuse &> /dev/null; then
reuse lint 2>&1 | head -50
else
echo "reuse tool not available"
# Check what files have SPDX headers
rg "SPDX-License-Identifier" . --type nix
fiRepository: rainlanguage/rainix
Length of output: 87
🏁 Script executed:
# Check README.md for license information
cat README.md | head -30Repository: rainlanguage/rainix
Length of output: 819
🏁 Script executed:
# Let's verify the git rev-parse behavior claim more thoroughly
# Check the context of how commit variable is used
sed -n '337,365p' flake.nixRepository: rainlanguage/rainix
Length of output: 1350
🏁 Script executed:
# Check if there are any other short hash uses
rg "short\|rev-parse" flake.nix -B2 -A2Repository: rainlanguage/rainix
Length of output: 45
🏁 Script executed:
# Let me verify the technical claim about git short hashes one more time
# by understanding how Goldsky deployment names work
sed -n '330,355p' flake.nixRepository: rainlanguage/rainix
Length of output: 1361
🏁 Script executed:
# Check what subgraph_deploy_version function does
cat lib/subgraph.sh | head -50Repository: rainlanguage/rainix
Length of output: 814
🏁 Script executed:
# Check if flake.nix needs REUSE license compliance
# According to coding guidelines, .nix files must comply with DCL-1.0
# Let's see if flake.nix has proper REUSE headers
head -3 flake.nix && echo "---" && tail -3 flake.nixRepository: rainlanguage/rainix
Length of output: 139
🏁 Script executed:
# Verify the exact behavior of git rev-parse --short
# The issue is that short hash length can vary based on object database size
# Let's confirm the proposed solution is correct
git rev-parse HEAD 2>&1 | head -1 | wc -cRepository: rainlanguage/rainix
Length of output: 65
Use the full commit hash instead of short hash for deterministic versioning.
git rev-parse --short HEAD produces variable-length abbreviated hashes that grow as the repository object database expands. This means the same commit can abbreviate differently across clones or later in repository history, causing the generated deployment version to change unexpectedly. Since subgraph_deploy_version hashes the address and commit together, any variation in the commit string will produce a different version identifier, breaking the deployment lookup and preventing the script from recognizing existing Goldsky deployments. This undermines the determinism required for idempotent deployments.
Proposed fix
- commit="$(${pkgs.git}/bin/git rev-parse --short HEAD)"
+ commit="$(${pkgs.git}/bin/git rev-parse HEAD)"📝 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.
| commit="$(${pkgs.git}/bin/git rev-parse --short HEAD)" | |
| for network in $(subgraph_networks ./subgraph/networks.json); do | |
| address=$(subgraph_network_address ./subgraph/networks.json "$network") | |
| version=$(subgraph_deploy_version "$address" "$commit") | |
| commit="$(${pkgs.git}/bin/git rev-parse HEAD)" | |
| for network in $(subgraph_networks ./subgraph/networks.json); do | |
| address=$(subgraph_network_address ./subgraph/networks.json "$network") | |
| version=$(subgraph_deploy_version "$address" "$commit") |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@flake.nix` around lines 337 - 340, The commit variable currently uses an
abbreviated, variable-length hash; change the assignment so commit="$(
${pkgs.git}/bin/git rev-parse HEAD )" (i.e., use the full commit hash) so
subgraph_deploy_version receives a deterministic commit string; update the line
that defines commit and keep the rest (calls to subgraph_networks,
subgraph_network_address, and subgraph_deploy_version) unchanged.
| echo "Building subgraph for $network..." | ||
| (cd ./subgraph && ${the-graph}/bin/graph build --network "$network") |
There was a problem hiding this comment.
The "already deployed" path still builds locally.
Moving only graph build under this branch still leaves Lines 334-335 (forge build and graph codegen) before the Goldsky existence check. An already-deployed version can still spend time building, or fail before it ever reaches the skip.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@flake.nix` around lines 346 - 347, The current script still runs "forge
build" and "graph codegen" before checking the Goldsky existence, so
already-deployed networks still perform local builds; modify the control flow so
the Goldsky existence check executes before any build steps and move both "forge
build" and "graph codegen" (and the subgraph build line "(cd ./subgraph &&
${the-graph}/bin/graph build --network \"$network\")") into the branch that runs
only when the network is not already deployed, ensuring the early-exit/skip path
bypasses all build commands.
|
@coderabbitai assess this PR size classification for the totality of the PR with the following criterias and report it in your comment: S/M/L PR Classification Guidelines:This guide helps classify merged pull requests by effort and complexity rather than just line count. The goal is to assess the difficulty and scope of changes after they have been completed. Small (S)Characteristics:
Review Effort: Would have taken 5-10 minutes Examples:
Medium (M)Characteristics:
Review Effort: Would have taken 15-30 minutes Examples:
Large (L)Characteristics:
Review Effort: Would have taken 45+ minutes Examples:
Additional Factors to ConsiderWhen deciding between sizes, also consider:
Notes:
|
|
Tip For best results, initiate chat on the files or code changes. SIZE=M |
1 participant
Summary
lib/subgraph.sh(shared between nix tasks and bats tests)sha256(contract_address + git_commit)[:8]network-listnix variable, read networks fromsubgraph/networks.jsoninsteadjqtonode-build-inputs.gitignoreentries for subgraph build artifactsGOLDSKY_NAME_AND_VERSION->GOLDSKY_SUBGRAPH_NAME(version is auto-derived)Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
Tests
Chores