feat: factor out all scripts and add ci for repo by rubydusa · Pull Request #30 · BreadchainCoop/etherform

rubydusa · 2026-03-31T09:15:31Z

No description provided.

Copilot

Pull request overview

This PR factors previously inlined workflow logic into reusable Bash scripts, adds a scripts test suite + CI workflow, and updates reusable workflows to fetch/run the shared scripts from the BreadchainCoop/etherform repository.

Changes:

Added modular scripts for deploy helpers, coverage parsing/threshold checks, and upgrade safety validation.
Added Bash-based unit tests and fixtures for the scripts plus a GitHub Actions workflow to run ShellCheck + tests.
Updated reusable workflows to checkout etherform scripts via etherform-ref and execute them instead of duplicating inline shell logic.

Reviewed changes

Copilot reviewed 21 out of 23 changed files in this pull request and generated 12 comments.

Show a summary per file

File	Description
tests/test-resolve-network.sh	Adds jq-level tests for network resolution fixtures.
tests/test-prepare-env.sh	Adds tests for env normalization and exporting.
tests/test-parse-broadcast.sh	Adds tests for broadcast parsing and summary generation.
tests/test-extract-summary.sh	Adds tests for coverage parsing + markdown output.
tests/test-deployment-summary.sh	Adds tests for deployment step summary formatting.
tests/test-check-threshold.sh	Adds tests for threshold pass/fail logic.
tests/fixtures/deploy-networks.json	Fixture for chain/network mapping.
tests/fixtures/coverage-raw.txt	Fixture for forge coverage output parsing.
tests/fixtures/broadcast/Deploy.s.sol/31337/run-latest.json	Fixture for Foundry broadcast parsing.
scripts/upgrade-safety/validate.sh	New script implementing upgrade safety validation formerly in workflows.
scripts/deploy/verify-blockscout.sh	New script to submit + poll contract verification on Blockscout.
scripts/deploy/resolve-network.sh	New script to resolve Blockscout URL/name and validate API reachability.
scripts/deploy/prepare-env.sh	New (sourceable) script for PRIVATE_KEY normalization + env exports.
scripts/deploy/parse-broadcast.sh	New script to find broadcast, extract chain id, and write summary file.
scripts/deploy/deployment-summary.sh	New script to emit deployment markdown to GitHub step summary.
scripts/coverage/extract-summary.sh	New script to parse coverage-raw.txt and produce outputs + markdown.
scripts/coverage/check-threshold.sh	New script to fail CI if coverage drops below threshold.
README.md	Documents `etherform-ref` and new scripts layout/testing.
.github/workflows/test.yml	Adds CI job to run ShellCheck + script unit tests.
.github/workflows/_upgrade-safety.yml	Switches workflow to checkout etherform scripts and call validate script.
.github/workflows/_foundry-cicd.yml	Replaces inline deploy/coverage/upgrade logic with checked-out scripts.
.github/workflows/_deploy-testnet.yml	Replaces inline deploy logic with checked-out scripts.
.github/workflows/_ci.yml	Replaces inline coverage logic with checked-out scripts.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-31T16:12:16Z

+  echo "::warning::No Blockscout URL found for chain ID $CHAIN_ID in $NETWORK_CONFIG. Falling back to testnets[0]."
+  BLOCKSCOUT_URL=$(jq -r '.testnets[0].blockscout_url' "$NETWORK_CONFIG")
+fi
+


After the fallback, BLOCKSCOUT_URL can still be empty/null (e.g., missing/empty testnets, missing key). That currently surfaces later as a confusing API error (e.g., null/api/). Add a post-fallback validation (non-empty and ^https?://) and emit a targeted error like “No valid blockscout_url found in ”.

Suggested change

# Post-fallback validation: ensure we have a non-empty, HTTP(S) Blockscout URL

if [[ -z "$BLOCKSCOUT_URL" || "$BLOCKSCOUT_URL" == "null" || ! "$BLOCKSCOUT_URL" =~ ^https?:// ]]; then

echo "::error::No valid blockscout_url found in $NETWORK_CONFIG"

exit 1

fi

Copilot · 2026-03-31T16:12:16Z

+if [[ -n "${DEPLOY_ENV_VARS:-}" ]]; then
+  while IFS= read -r line; do
+    [[ -z "$line" || "$line" == \#* ]] && continue
+    export "${line?}"


Directly export \"${line?}\" executes arbitrary assignments from DEPLOY_ENV_VARS without validating the key. A malformed line (or a line starting with -) can cause export errors and halt the calling workflow (since this is sourced). Prefer parsing KEY=VALUE, validating KEY against a safe pattern (e.g., ^[A-Za-z_][A-Za-z0-9_]*$), and then exporting only that key/value.

Suggested change

export "${line?}"

# Expect KEY=VALUE; skip malformed lines

if [[ "$line" != *=* ]]; then

continue

fi

key=${line%%=*}

value=${line#*=}

# Validate KEY against a safe variable-name pattern

if [[ ! "$key" =~ ^[A-Za-z_][A-Za-z0-9_]*$ ]]; then

continue

fi

export "$key=$value"

Copilot · 2026-03-31T16:12:17Z

+      echo "::warning::Failed to build base branch — contracts without explicit reference will be validated for upgradeability only"
+    fi
+  else
+    echo "::warning::Could not checkout base branch '$BASE_BRANCH' — contracts without explicit reference will be validated for upgradeability only"


If git worktree add fails, BASE_DIR remains a plain temp directory and git worktree remove won’t delete it (and errors are suppressed), leaving leaked temp dirs on the runner. Track whether the worktree was successfully added, and rm -rf \"$BASE_DIR\" in the failure path (and/or after attempting git worktree remove).

Suggested change

echo "::warning::Could not checkout base branch '$BASE_BRANCH' — contracts without explicit reference will be validated for upgradeability only"

echo "::warning::Could not checkout base branch '$BASE_BRANCH' — contracts without explicit reference will be validated for upgradeability only"

rm -rf "$BASE_DIR"

Copilot · 2026-03-31T16:12:17Z

+
+# Clean up worktree
+if [[ -n "$BASE_DIR" ]]; then
+  git worktree remove "$BASE_DIR" --force 2>/dev/null || true


If git worktree add fails, BASE_DIR remains a plain temp directory and git worktree remove won’t delete it (and errors are suppressed), leaving leaked temp dirs on the runner. Track whether the worktree was successfully added, and rm -rf \"$BASE_DIR\" in the failure path (and/or after attempting git worktree remove).

Suggested change

git worktree remove "$BASE_DIR" --force 2>/dev/null || true

git worktree remove "$BASE_DIR" --force 2>/dev/null || true

rm -rf "$BASE_DIR" 2>/dev/null || true

Copilot · 2026-03-31T16:12:17Z

+# Validate each contract
+while IFS= read -r entry; do
+  CONTRACT=$(echo "$entry" | jq -r '.contract')
+  REF_VALUE=$(echo "$entry" | jq -c '.reference // empty')


The repo adds a scripts test harness, but scripts/upgrade-safety/validate.sh isn’t covered. Given its branching behavior (missing config, empty contracts, base-branch worktree build, explicit reference handling), add at least smoke tests using small fixture configs and stubbing git/forge/npx via PATH to validate the decision logic and emitted step-summary output.

Copilot · 2026-03-31T16:12:18Z

 | `run-halmos` | boolean | `false` | Run Halmos symbolic execution |
+| `etherform-ref` | string | `'main'` | Git ref for etherform scripts checkout |

 | Secret | Required | Description |


etherform-ref is documented under the “Secret” table here, but it’s an input (non-secret) in the workflows. Update the README so etherform-ref appears only in the inputs section(s), and avoid listing it as a secret to prevent confusion for workflow consumers.

Copilot · 2026-03-31T16:12:18Z

 | `node-version` | string | `'20'` | Node.js version for package installation |
 | `upgrades-config` | string | `'.github/upgrades.json'` | Path to upgrade safety config |
 | `base-branch` | string | `'main'` | Base branch for upgrade safety comparison |
+| `etherform-ref` | string | `'main'` | Git ref for etherform scripts checkout |


etherform-ref is documented under the “Secret” table here, but it’s an input (non-secret) in the workflows. Update the README so etherform-ref appears only in the inputs section(s), and avoid listing it as a secret to prevent confusion for workflow consumers.

- Add post-fallback validation for BLOCKSCOUT_URL in resolve-network.sh - Validate KEY=VALUE format and key names in prepare-env.sh export - Add curl timeouts and normalize URL in verify-blockscout.sh status poll

- Hard fail when base branch build fails (opt-in ALLOW_FALLBACK=true) - Pin @openzeppelin/upgrades-core version (default 1.44.2) - Require upgrade-safety success for deploy (no longer allows skipped) - Quote deploy-script input to prevent flag injection - Fail on empty contracts array when base branch has entries - Remove duplicate Node.js setup steps in both workflows

- validate.sh: fail hard when base branch fetch/read fails during empty-contracts check (gated by ALLOW_FALLBACK=true) - verify-blockscout.sh: normalize BLOCKSCOUT_URL once and use consistently for both submission and polling - test-prepare-env.sh: add tests for malformed lines, invalid key names, and values containing equals signs

…dening-fixes fix: address PR #32 review comments

fix: harden deploy scripts and upgrade safety workflows

Copilot

Pull request overview

Copilot reviewed 22 out of 24 changed files in this pull request and generated 9 comments.

Comments suppressed due to low confidence (1)

.github/workflows/_foundry-cicd.yml:88

The workflow_call input run-upgrade-safety was removed, which is a breaking change for any repos currently passing that input (they’ll get an “Unexpected input” workflow error). If disabling upgrade-safety is still a supported use case, consider reintroducing the input (even if deprecated) or documenting the breaking change prominently.

        default: 0

      # Upgrade Safety Options
      upgrades-config:
        description: 'Path to upgrade safety configuration (upgrades.json)'
        type: string
        default: '.github/upgrades.json'
      main-branch:
        description: 'Base branch for upgrade safety comparison'
        type: string
        default: 'main'

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

RonTuretzky · 2026-04-08T17:46:12Z

@rubydusa is this ready for review ?

rubydusa · 2026-04-08T19:10:25Z

@rubydusa is this ready for review ?

Yes

RonTuretzky

@hudsonhrh give this a review when you get a chance 👍

hudsonhrh

@rubydusa @RonTuretzky thoughts here?

hudsonhrh · 2026-04-13T17:34:40Z

+      etherform-ref:
+        description: 'Git ref for etherform scripts checkout (default: main)'
+        type: string
+        default: 'main'


should we recommend main by default or a specific commit? specific commit is safer so if we merge something to main they have to chose to upgrade so we can't rug them or something

make a backlog issue and address in a future release

should we recommend main by default or a specific commit? specific commit is safer so if we merge something to main they have to chose to upgrade so we can't rug them or something

ideally we'll have releases with tags or something, if you look at other github actions they usually have something like a vX appended

Opening an issue

rubydusa added 4 commits March 31, 2026 12:14

feat: factor out all scripts and add ci for repo

7751388

fix: shellcheck lint

20843e2

feat: add etherform-ref

a31a8b9

docs: add etherform-ref explanation to README.md

f13b06a

rubydusa mentioned this pull request Mar 31, 2026

refactor: replace submodule with local testing framework #15

Closed

RonTuretzky requested a review from Copilot March 31, 2026 16:08

Copilot AI reviewed Mar 31, 2026

View reviewed changes

fix: harden deploy scripts against invalid inputs

60e77d3

- Add post-fallback validation for BLOCKSCOUT_URL in resolve-network.sh - Validate KEY=VALUE format and key names in prepare-env.sh export - Add curl timeouts and normalize URL in verify-blockscout.sh status poll

RonTuretzky mentioned this pull request Mar 31, 2026

fix: harden deploy scripts and upgrade safety workflows #32

Merged

7 tasks

RonTuretzky added 2 commits March 31, 2026 13:43

Base automatically changed from Rubydusa/saving-circles-with-upgrade-safety-no-baseline to main April 2, 2026 08:10

rubydusa and others added 4 commits April 2, 2026 11:37

fix: address Rubydusa PR comments

18c821a

fix: minor shellcheck issue

d477c64

Merge pull request #33 from BreadchainCoop/Rubydusa/deploy-script-har…

5a5e8b7

…dening-fixes fix: address PR #32 review comments

Merge pull request #32 from BreadchainCoop/fix/deploy-script-hardening

1e3538a

fix: harden deploy scripts and upgrade safety workflows

RonTuretzky requested a review from Copilot April 6, 2026 20:26

Copilot started reviewing on behalf of RonTuretzky April 6, 2026 20:27 View session

Copilot AI reviewed Apr 6, 2026

View reviewed changes

rubydusa marked this pull request as ready for review April 8, 2026 19:10

rubydusa changed the title ~~[DRAFT] feat: factor out all scripts and add ci for repo~~ feat: factor out all scripts and add ci for repo Apr 9, 2026

RonTuretzky reviewed Apr 10, 2026

View reviewed changes

hudsonhrh requested changes Apr 13, 2026

View reviewed changes

RonTuretzky self-requested a review April 13, 2026 19:54

RonTuretzky approved these changes Apr 13, 2026

View reviewed changes

rubydusa merged commit bb0e3ef into main Apr 13, 2026
6 checks passed

rubydusa deleted the Rubydusa/ci-testing branch April 13, 2026 20:12

+# Post-fallback validation: ensure we have a non-empty, HTTP(S) Blockscout URL
+if [[ -z "$BLOCKSCOUT_URL" || "$BLOCKSCOUT_URL" == "null" || ! "$BLOCKSCOUT_URL" =~ ^https?:// ]]; then
+  echo "::error::No valid blockscout_url found in $NETWORK_CONFIG"
+  exit 1
+fi

-    export "${line?}"
+    # Expect KEY=VALUE; skip malformed lines
+    if [[ "$line" != *=* ]]; then
+      continue
+    fi
+    key=${line%%=*}
+    value=${line#*=}
+    # Validate KEY against a safe variable-name pattern
+    if [[ ! "$key" =~ ^[A-Za-z_][A-Za-z0-9_]*$ ]]; then
+      continue
+    fi
+    export "$key=$value"

	echo "::warning::Could not checkout base branch '$BASE_BRANCH' — contracts without explicit reference will be validated for upgradeability only"
	echo "::warning::Could not checkout base branch '$BASE_BRANCH' — contracts without explicit reference will be validated for upgradeability only"
	rm -rf "$BASE_DIR"

	git worktree remove "$BASE_DIR" --force 2>/dev/null \|\| true
	git worktree remove "$BASE_DIR" --force 2>/dev/null \|\| true
	rm -rf "$BASE_DIR" 2>/dev/null \|\| true

Conversation

rubydusa commented Mar 31, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 31, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

RonTuretzky commented Apr 8, 2026

Uh oh!

rubydusa commented Apr 8, 2026

Uh oh!

RonTuretzky left a comment

Choose a reason for hiding this comment

Uh oh!

hudsonhrh left a comment

Choose a reason for hiding this comment

Uh oh!

hudsonhrh Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

RonTuretzky Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

rubydusa Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

rubydusa Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants