Skip to content

fix: address PR #32 review comments#33

Merged
RonTuretzky merged 2 commits into
fix/deploy-script-hardeningfrom
Rubydusa/deploy-script-hardening-fixes
Apr 6, 2026
Merged

fix: address PR #32 review comments#33
RonTuretzky merged 2 commits into
fix/deploy-script-hardeningfrom
Rubydusa/deploy-script-hardening-fixes

Conversation

@rubydusa
Copy link
Copy Markdown
Contributor

@rubydusa rubydusa commented Apr 2, 2026

Summary

Addresses review comments from @rubydusa on PR #32.

  • Remove run-upgrade-safety input — upgrade safety always runs; removes the issue where disabling it implicitly blocked testnet deployment
  • Simplify Node.js cache setupcache: ${{ inputs.package-manager }} instead of ternary expression; setup-node ignores invalid cache values
  • prepare-env.sh fails on malformed lines — lines without = or with invalid key names now error instead of being silently skipped
  • Extract config validation to validate-config.sh — checks that no .contract entries were removed compared to the base branch (subset check by contract name, not just count)
  • Simplify validate.sh — empty contracts array is a simple skip; complex base-branch comparison logic moved to the dedicated config validation script; removed ALLOW_FALLBACK
  • Hard fail on base branch errors — when contracts without explicit references exist, fetch/checkout/build failures are errors, not warnings

Test plan

  • shellcheck passes on all scripts
  • All unit tests pass (bash tests/test-*.sh)
  • Verify upgrade-safety runs unconditionally (no run-upgrade-safety toggle)
  • Verify validate-config.sh catches removed contracts
  • Verify prepare-env.sh fails on malformed DEPLOY_ENV_VARS

🤖 Generated with Claude Code

@rubydusa rubydusa changed the base branch from main to fix/deploy-script-hardening April 2, 2026 08:42
@rubydusa rubydusa requested a review from RonTuretzky April 2, 2026 08:50
@RonTuretzky RonTuretzky merged commit 5a5e8b7 into fix/deploy-script-hardening Apr 6, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants