Skip to content

ci: drop node.dev sanitization step (after genlayer-node NOD-973) #430

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
dohernandez wants to merge 2 commits into
base: main
Choose a base branch
from
Open

ci: drop node.dev sanitization step (after genlayer-node NOD-973) #430

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fccf71b
ci: drop node.dev sanitization step
dohernandez Jun 29, 2026
48b274c
docs: mention provider in sanitize-config one-line summary
dohernandez Jun 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions .github/scripts/sanitize-config.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,11 @@ fi
echo "Sanitizing config file: $CONFIG_FILE"

# Use yq to:
# 1. Replace RPC URLs with TODO placeholders
# 2. Delete node.dev section
# Replace RPC URLs / provider with TODO placeholders
yq -i '
.rollup.genlayerchainrpcurl = "TODO: Set your GenLayer Chain ZKSync HTTP RPC URL here" |
.rollup.genlayerchainwebsocketurl = "TODO: Set your GenLayer Chain ZKSync WebSocket RPC URL here" |
.rollup.provider = "TODO: Set your GenLayer Chain ZKSync provider" |
del(.node.dev)
.rollup.provider = "TODO: Set your GenLayer Chain ZKSync provider"
Comment on lines +23 to +27

@coderabbitai coderabbitai Bot Jun 29, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Fail fast if upstream still ships node.dev.

.github/actions/sync-files/action.yml:40-60 always syncs this script’s output downstream, so once del(.node.dev) is removed, merging this early will silently publish a config that still contains node.dev. Please enforce the sequencing requirement in code by aborting when .node.dev is still present instead of relying on merge timing alone.

🤖 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 @.github/scripts/sanitize-config.sh around lines 23 - 27, The sanitize-config
script currently assumes `.node.dev` has already been removed, but
`sanitize-config.sh` can still run while that field exists and then propagate an
invalid config. Update the script to explicitly check the input config for
`.node.dev` before the `yq` replacements run and abort with a clear failure if
it is still present. Use the existing `yq`-based flow in `sanitize-config.sh`
and keep the guard close to the current placeholder replacement logic so the
sequencing requirement is enforced in code rather than by merge order.

@dohernandez dohernandez Jun 29, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fine, it is set in the PR description

@coderabbitai coderabbitai Bot Jun 29, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dohernandez Understood, thanks for clarifying. Since the sequencing requirement is already captured in the PR description, I won’t push further on adding the in-script guard in this PR.

' "$CONFIG_FILE"

echo "Config sanitization completed"
5 changes: 2 additions & 3 deletions .github/workflows/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -181,13 +181,12 @@ The workflow uses composite actions for code reusability:
- `.github/scripts/git-utils.sh` - Branch creation, commit, and push operations
- `.github/scripts/version-utils.sh` - Version detection and validation
- `.github/scripts/doc-generator.sh` - Wrapper for npm documentation generation
- `.github/scripts/sanitize-config.sh` - Sanitizes config file (replaces URLs, removes dev section)
- `.github/scripts/sanitize-config.sh` - Sanitizes config file (replaces RPC URLs and provider)
- `.github/scripts/sanitize-docker-compose.sh` - Removes alloy service and volumes from docker-compose

### Config Sanitization
The config sync process includes automatic sanitization using `yq`:
- **URL Replacement**: RPC URLs replaced with TODO placeholders
- **Dev Section Removal**: `node.dev` section is removed
- **URL Replacement**: RPC URLs and provider replaced with TODO placeholders

### Docker Compose Sanitization
The docker-compose sync process includes automatic sanitization using `yq` and `sed`:
Expand Down
Loading