Skip to content

feat(scripts): add dev-cluster node upgrade tooling - #4100

Open
SimonRastikian wants to merge 8 commits into
mainfrom
3934-dev-cluster-node-upgrade
Open

feat(scripts): add dev-cluster node upgrade tooling#4100
SimonRastikian wants to merge 8 commits into
mainfrom
3934-dev-cluster-node-upgrade

Conversation

@SimonRastikian

Copy link
Copy Markdown
Contributor

Deals partly with #3934 namely the nodes upgrade on the dev cluster

@claude

claude Bot commented Aug 7, 2026

Copy link
Copy Markdown

PR title type suggestion: This PR adds development/operations infrastructure scripts rather than user-facing features, so the type prefix should probably be chore: instead of feat:.

Suggested title: chore(scripts): add dev-cluster node upgrade tooling

@claude

claude Bot commented Aug 7, 2026

Copy link
Copy Markdown

Pull request overview

Adds an interactive Bash ops toolkit under scripts/ops/: a top-level menu.sh, a shared common.sh helper library, and a dev-cluster/ flow that rolls a published release out to a NEAR One dev cluster by rewriting each mpc-node-* Nomad job's image over the Nomad HTTP API (plan, confirm, register, wait), then verifies node build-info and offers an on-chain test signature. RELEASES.md gains an "Ops tooling" section describing the entry point.

The credential handling is thoughtful — auth goes through curl's config stream rather than argv, nomad_auth_state() never echoes the secret, and every write is confirm-gated. The issues below are in the rollout loop's failure semantics, the forced plaintext transport, and documentation that describes a step this PR does not implement.

Changes:

  • New scripts/ops/common.sh: colour/TTY handling, die/step/ok/warn, require_cmds, check_version, confirm, command echoing (fmt_cmd/show_cmd/show_output/run_cmd/run_step), and an unused sha256_of.
  • New scripts/ops/dev-cluster/dev-common.sh: per-network cluster constants, prompts for Nomad IP / basic auth / metrics addresses, verify_nodes build-info check, and test_sign on-chain smoke test.
  • New scripts/ops/dev-cluster/migrate-dev-nodes.sh: Nomad API client, job image rewrite via jq, plan gate on FailedTGAllocs, register, and allocation wait; optional skopeo tag-existence pre-check.
  • New scripts/ops/dev-cluster/dev-menu.sh and scripts/ops/menu.sh: interactive drivers.
  • RELEASES.md: new "Ops tooling" section.

Reviewed changes

Per-file summary
File Description
RELEASES.md Adds an "Ops tooling" section documenting menu.sh and the dev-cluster upgrade flow
scripts/ops/common.sh New generic helper library sourced by all ops scripts
scripts/ops/menu.sh New top-level interactive menu (prepare release / migrate dev cluster)
scripts/ops/dev-cluster/dev-common.sh New dev-cluster constants, prompts, node verification, and on-chain test sign
scripts/ops/dev-cluster/dev-menu.sh New dev-cluster driver: network + version, node upgrade, verify, test sign
scripts/ops/dev-cluster/migrate-dev-nodes.sh New Nomad-API job image swap with plan/confirm/register/wait

Findings

Blocking (must fix before merge):

  • scripts/ops/dev-cluster/migrate-dev-nodes.sh:71the rollout continues after a node fails to come back. On timeout wait_for_alloc calls warn and falls off the end, so it returns 0; upgrade_nomad_job (line 128) returns 0; the loop at lines 152–154 proceeds to register the next mpc-node-* job. The comment at line 127 ("This node must be back before the caller moves to the next.") states an invariant the code does not enforce — and on a 2-node dev cluster, taking the second node down while the first is still not running drops the cluster below threshold. Make the timeout fatal, or gate continuation on the operator by following the warn with a confirm "Continue to the next job anyway?" || die .... Note that die inside upgrade_nomad_job already aborts the loop (lines 117/120/125), so the failure mode is inconsistent today: a plan failure stops the rollout, a node that never comes back does not.

  • scripts/ops/dev-cluster/dev-common.sh:42,47a pasted https:// URL is silently downgraded to plain HTTP. Line 42 strips both schemes and line 47 unconditionally re-prefixes http://, and there is no interactive way to select HTTPS. The very next thing the toolkit does is send basic-auth credentials (migrate-dev-nodes.sh:33) and optionally an X-Nomad-Token header (line 34) over that connection — base64/cleartext on the wire. This undercuts the argv-hygiene care taken elsewhere in the same function. Capture whether the operator typed https before stripping the scheme and use it when rebuilding NOMAD_ADDR. If plaintext really is the only option for these clusters, say so in the prompt and warn when credentials are about to be sent unencrypted, rather than rewriting the operator's input without telling them.

  • scripts/ops/menu.sh:25 and scripts/ops/dev-cluster/dev-menu.sh:61the UI advertises a contract migration step that does not exist. Menu entry 2 reads "upgrade a dev cluster's nodes, verify, then its contract", and dev-menu.sh labels its only rollout step ### Step 1 — nodes, but no contract migration is implemented anywhere in scripts/ops/ and dev-menu.sh ends after verify + test sign. Per CLAUDE.md's documentation-alignment rule this is review-blocking: either drop "then its contract" and the "Step 1" numbering, or land them behind a TODO(#NNNN) that names the follow-up issue.

  • RELEASES.md:173-176the documented flow omits the on-chain transaction it actually sends. dev-menu.sh:71 always runs test_sign, which submits a real sign call with an attached deposit (1 NEAR on testnet, 0.1 NEAR on mainnet) from an MPC node's own member account (dev-common.sh:112-122). The prose stops at "checks the nodes report the new release=". An operator reading only RELEASES.md will not expect a mainnet transaction spending a node account's balance. Document the test-sign step, the deposit, the signing account, and the MPC_SIGN_WITH override.

Non-blocking (nits, follow-ups, suggestions):

  • scripts/ops/dev-cluster/migrate-dev-nodes.sh:27,29 — the comment at line 31 explains that auth avoids argv because /proc/<pid>/cmdline is readable, but --data "$data" puts the entire job definition on the command line. Nomad mpc-node job specs routinely carry secrets in Env/template blocks, so the same exposure applies to material that matters more. The echoed command at line 29 already claims --data @-; consider making that true by folding the body into the config stream (-K with a process substitution plus --data-binary @- works, as does an umask 077 temp file).
  • scripts/ops/dev-cluster/migrate-dev-nodes.sh:33 — the value interpolated into curl's config is not escaped. curl parses user = "..." as a quoted string with backslash escapes, so a password containing a double quote or a backslash is truncated or mangled into a confusing 401. Escape backslashes and then double quotes before interpolating.
  • scripts/ops/dev-cluster/migrate-dev-nodes.sh:148/v1/jobs?prefix=mpc-node only lists the default namespace; NOMAD_NAMESPACE is not honoured (that is a CLI feature, not an API one). If any dev cluster uses a namespace, the script dies with "No mpc-node-* jobs found" and no hint why. Appending a namespace= query param and threading it through nomad_curl would cover it.
  • scripts/ops/dev-cluster/dev-common.sh:83-105verify_nodes polls once, immediately after the last allocation reports running, so a node still warming up produces a spurious "Not all nodes are on X yet". A short retry (a few attempts, 5s apart) would make the check trustworthy enough to act on.
  • scripts/ops/common.sh:43sha256_of is defined but never called by anything in this PR. Drop it until there is a caller.
  • scripts/ops/common.sh:37 vs scripts/ops/prepare-github-release.sh:39 — divergent semver regexes. menu.sh:16 accepts 3.14.0-rc1, then prepare-github-release.sh rejects it with a different message a second later. Relatedly, prepare-github-release.sh still carries its own die/require_cmds (lines 46-60) instead of sourcing the new common.sh; consolidating would remove the divergence.
  • RELEASES.md:179-180 — "Nothing cluster-specific is stored in this repo" is contradicted by dev-common.sh:20-25, which hardcodes mpc-dev-contract.testnet, dev-contract.near, and four member account IDs. The claim holds for network addresses and credentials only; narrow the sentence to match.
  • scripts/ops/dev-cluster/dev-common.sh:88local ... ok=0 sits next to calls to the ok() function on line 101. Bash keeps function and variable namespaces separate so this works, but it reads as a bug; renaming the counter to matched would save the next reader the lookup.
  • scripts/ops/dev-cluster/dev-common.sh:93fmt_cmd quotes the literal pipe character, so the echoed line is not a runnable pipeline despite the "copy-pasteable" promise on common.sh:59. Minor, but the two-curl-calls-per-node shape exists only to produce that echo.
  • No shellcheck job exists in .github/workflows/, yet these files carry # shellcheck source= directives implying it is expected to run. ~490 new lines of Bash with no lint gate is worth a follow-up.
  • scripts/ops/common.sh:87# Print a command, run it, let its output through. paraphrases the two lines below it; per CLAUDE.md's comment policy this one can go. Most other comments in the PR correctly explain why and should stay.

⚠️ Issues found

@SimonRastikian

SimonRastikian commented Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

This one is extremely difficult to test because actual e2e tests require connection to the real IP

@SimonRastikian
SimonRastikian marked this pull request as draft August 10, 2026 09:57
Base automatically changed from 3934-rename-prepare-github-release to main August 10, 2026 10:20
@SimonRastikian
SimonRastikian force-pushed the 3934-dev-cluster-node-upgrade branch from de45aee to b1ae09b Compare August 10, 2026 10:20
SimonRastikian and others added 4 commits August 10, 2026 13:10
migrate-dev-nodes.sh now takes the network as its first argument and, before
the image swap, pulls MPC_ACCOUNT_ID/MPC_ACCOUNT_SK from each job's Env and
imports any key missing from the local near-cli keystore. The secret is
masked in the echoed command. test_sign keeps a manual interactive import
(ensure_signing_key) as fallback.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@SimonRastikian
SimonRastikian force-pushed the 3934-dev-cluster-node-upgrade branch from 1584c50 to d561db9 Compare August 10, 2026 13:46
@SimonRastikian SimonRastikian self-assigned this Aug 12, 2026
@SimonRastikian
SimonRastikian marked this pull request as ready for review August 12, 2026 14:05
@SimonRastikian

Copy link
Copy Markdown
Contributor Author

@claude review

@claude

claude Bot commented Aug 12, 2026

Copy link
Copy Markdown

Pull request overview

Adds an interactive Bash ops toolkit under scripts/ops/: a top-level menu.sh, a shared common.sh, and a dev-cluster/ flow that rolls a published release onto a NEAR One dev cluster by rewriting each mpc-node-* Nomad job's image over the Nomad HTTP API (plan, confirm, register, wait), then verifies node build-info and offers an on-chain test signature. prepare-github-release.sh is refactored onto the shared helpers and RELEASES.md documents the entry point.

This revision addresses most of the earlier round: the alloc-wait timeout is operator-gated, HTTPS is preserved, NOMAD_NAMESPACE is threaded through, curl config values are escaped, verify_nodes retries, and the semver check is shared with release.yml. The new first-time-setup.sh is the significant addition, and it changes the toolkit's security posture: it copies each node's NEAR signer secret key out of Nomad onto the operator's machine. The findings below concern that key handling, one echo path that can print the same key to the terminal, and a regression from widening the semver check.

Changes:

  • New common.sh: TTY/colour handling, die/step/ok/warn, require_cmds, check_version, confirm, command echoing, sha256_of.
  • New dev-cluster/dev-common.sh: per-network cluster constants, Nomad/metrics prompts, verify_nodes with retries, test_sign.
  • New dev-cluster/first-time-setup.sh: reads MPC_ACCOUNT_ID/MPC_ACCOUNT_SK from Nomad job definitions and writes near-cli keystore files, deriving the ed25519 public half in an inline Python script.
  • New dev-cluster/migrate-dev-nodes.sh: Nomad API client (auth via curl config stream, body via stdin), jq image rewrite, plan gate on FailedTGAllocs, register, alloc wait, optional skopeo pre-check.
  • New dev-cluster/dev-menu.sh and menu.sh drivers; prepare-github-release.sh moved onto common.sh with comments condensed; RELEASES.md "Ops tooling" section.

Reviewed changes

Per-file summary
File Description
RELEASES.md New "Ops tooling" section: menu.sh, dev-cluster flow, env-var overrides, keystore import
scripts/ops/common.sh New generic helper library sourced by all ops scripts
scripts/ops/menu.sh New top-level interactive menu
scripts/ops/prepare-github-release.sh Refactored onto common.sh; comment block condensed
scripts/ops/dev-cluster/dev-common.sh Cluster constants, prompts, verify_nodes, test_sign
scripts/ops/dev-cluster/dev-menu.sh Dev-cluster driver: network + version, upgrade, verify, test sign
scripts/ops/dev-cluster/first-time-setup.sh Imports member signing keys from job definitions into the near-cli keystore
scripts/ops/dev-cluster/migrate-dev-nodes.sh Nomad-API job image swap: plan, confirm, register, wait; key import

Findings

Blocking (must fix before merge):

  • scripts/ops/dev-cluster/migrate-dev-nodes.sh:59,129the echoed plan response can print MPC_ACCOUNT_SK to the operator's terminal. Line 59 echoes the full body of every non-GET response and line 129 requests Diff: true. Nomad's plan diff is contextual — the API returns unchanged fields too (that is what nomad plan -verbose toggles: display of entries the API already sent) — so the diff carries each task's Env map, including the very MPC_ACCOUNT_SK this PR reads from the same job definition at first-time-setup.sh:14. Line 58 already declines to echo GET job bodies, but only for noise; the same content returns through the plan. show_output's 1500-char cap is not a security control, since truncation depends on field ordering. Note the diff is never read — lines 131-132 consume only .FailedTGAllocs and .Warnings — so Diff: false costs nothing, and the plan call should skip the automatic echo (a quiet flag on nomad_curl, or echo only the two parsed fields; >/dev/null will not do it, as show_output writes to stderr from inside nomad_curl). This is also what makes RELEASES.md:186-187 ("Keys never appear ... in the echoed output") false today.

  • scripts/ops/dev-cluster/first-time-setup.sh:69-80 and migrate-dev-nodes.sh:167-170every node's signer secret key is copied to the operator's machine unprompted. deployment/start.sh:72-84 shows MPC_ACCOUNT_SK becomes the node's near_signer_key and near_responder_keys[0], so a local plaintext copy can act as that node's on-chain account. The loop runs unconditionally for every mpc-node-* job, on every invocation, before any per-job confirmation, and on the mainnet cluster too — while only one account is ever used for signing (dev-common.sh:122, ${MEMBER_ACCOUNTS%% *}), and only by test_sign, which is itself confirm-gated at dev-common.sh:131 and may be declined. It also contradicts RELEASES.md:178-179 ("every write is behind a confirmation prompt"). Please (a) put the import behind confirm, naming what gets written and where, (b) import only the account test_sign will use, and (c) document that the operator ends up holding a copy of the node's key and how to remove it. Scoping to the single needed account also makes the mainnet path far less alarming.

  • scripts/ops/prepare-github-release.sh:31,73,77pre-release versions are now accepted but only half-supported, and the follow-up run fails misleadingly. check_version (common.sh:37) deliberately permits -SUFFIX, which this script previously rejected up front. The bump still handles bare X.Y.Z only: the awk pattern on line 73 requires a " immediately after the patch digits, so once version = "3.14.0-rc1" is committed, the next run (rc2 or final 3.14.0) matches nothing, OLD_VERSION is empty, and line 74 dies with Could not find a workspace 'version = "X.Y.Z"' line — pointing the operator at a malformed Cargo.toml instead of the unsupported format. The sed -E on line 77 has the same gap and would silently no-op. Either widen both patterns with an optional (-[A-Za-z0-9.]+)? group, or keep this script strict via its own check and leave the permissive regex to the workflow.

Non-blocking (nits, follow-ups, suggestions):

  • migrate-dev-nodes.sh:129,137 — the argv-hygiene fix is incomplete: the curl body now goes through stdin, but jq -n --argjson job "$updated" still puts the whole job definition, MPC_ACCOUNT_SK included, on jq's command line, where /proc/<pid>/cmdline is world-readable. jq '{Job: .}' <<<"$updated" is equivalent and keeps it off argv (and clear of MAX_ARG_STRLEN for large specs).
  • prepare-github-release.sh:81 — the condensed comment is inverted relative to the code: it reads as though the test fails when the ABI was unaffected, while line 84 dies precisely because the test passed. The replaced comment had it right. Line 70 likewise: macOS ships BSD sed/grep, so it is the GNU-only flags that are unavailable, and no grep remains in the annotated block.
  • migrate-dev-nodes.sh:148require_cmds curl jq near omits python3, which first-time-setup.sh:24 needs; a missing interpreter currently surfaces as Could not store key for <account> plus an opaque near-cli failure later.
  • migrate-dev-nodes.sh:40-41 — the curl >= 7.76 requirement for --fail-with-body is documented but unchecked; on older curl every request fails with option --fail-with-body: is unknown.
  • migrate-dev-nodes.sh:65-80 — every poll runs nomad_curl, which show_cmds to stderr, so the waiting for allocation ... dots are interleaved with up to 72 echoed curl lines and the eventual warn is buried. A quiet mode for polling GETs would help.
  • dev-common.sh:28 vs :30 — inconsistent override semantics: MPC_NODE_ADDRS_DEV_* uses ${!var:-} so an intentionally empty export still prompts, while NOMAD_HTTP_AUTH_DEV_* uses +set for exactly that case (and prompt_node_addrs:71 already honours +set).
  • dev-menu.sh:67-71verify_nodes ... || true is followed unconditionally by test_sign, so a cluster that failed the build-info check still gets an on-chain transaction with an attached deposit. Gating on verification (or a confirm that says verification failed) matches the "stoppable at any step" intent. test_sign could also check the already-available have_signing_key "$signer" first, rather than letting near-cli fail opaquely when MEMBER_ACCOUNTS has drifted from the accounts actually in the jobs.
  • first-time-setup.sh:50-52 — the error says "unexpected ed25519 key length" for any key, including a secp256k1: one where 32 bytes is correct; echoing the observed prefix would make it diagnosable.
  • Carried over from the previous round, still open: menu.sh:25 advertises "verify, then its contract" and dev-menu.sh:61 labels its only rollout step ### Step 1 — nodes, but no contract migration exists here (CLAUDE.md treats this drift as review-blocking — drop the wording or land it behind a TODO(#NNNN)); common.sh:43 sha256_of has no caller; there is still no shellcheck gate in .github/workflows/ despite the # shellcheck source= directives, now over ~600 lines of Bash.

No embedded instructions or prompt-injection attempts were found in the diff or PR metadata.

⚠️ Issues found

@claude

claude Bot commented Aug 12, 2026

Copy link
Copy Markdown

Pull request overview

Re-review after the 2026-08-10 updates. Three of the four previously blocking items are fixed: wait_for_alloc now gates continuation on the operator (and dies otherwise), prompt_nomad_ip preserves a pasted https:// scheme and warns before sending credentials over plain HTTP, and the request body travels via stdin with --data-binary @- instead of argv, with curl -K values escaped.

The new surface is first-time-setup.sh, which lifts each member account's MPC_ACCOUNT_SK out of the Nomad job definition and writes near-cli keystore files so test_sign can sign. That is where the remaining problems concentrate: it puts node signing keys on the operator's disk unprompted, and the plan-response echo it shares with nomad_curl can print those same keys to the terminal. The prepare-github-release.sh refactor onto the shared common.sh also widened the accepted version format beyond what the rest of that script handles, and inverted one comment's meaning.

Changes:

  • New first-time-setup.sh: jq-extracts MPC_ACCOUNT_ID/MPC_ACCOUNT_SK per MPC task, derives the ed25519 public half from the secret key in inline Python, writes both legacy-keychain files at 0600.
  • migrate-dev-nodes.sh: --fail-with-body, curl -K config-stream credentials with escaping, NOMAD_NAMESPACE in the query string, body via stdin, confirm-gated allocation timeout, plus a pre-rollout key-import pass over every discovered job.
  • dev-common.sh: HTTPS-preserving IP prompt + plaintext warning, have_signing_key, verify_nodes retries 3x/5s, MEMBER_ACCOUNTS trimmed to two per network, MPC_SIGN_WITH override.
  • prepare-github-release.sh: sources common.sh for die/require_cmds/check_version; several comments compressed.
  • RELEASES.md: "Ops tooling" section extended to describe the key import.

Reviewed changes

Per-file summary
File Description
RELEASES.md Ops tooling section; now also documents the key import
scripts/ops/common.sh Shared helpers: output, require_cmds, check_version, confirm, command echoing
scripts/ops/menu.sh Top-level interactive menu
scripts/ops/dev-cluster/dev-common.sh Cluster constants, prompts, have_signing_key, retrying verify_nodes, test_sign
scripts/ops/dev-cluster/dev-menu.sh Dev-cluster driver: network + version, upgrade, verify, test sign
scripts/ops/dev-cluster/first-time-setup.sh Imports member signing keys from job definitions into ~/.near-credentials
scripts/ops/dev-cluster/migrate-dev-nodes.sh Nomad API client and job image swap: plan, confirm, register, wait
scripts/ops/prepare-github-release.sh Reuses common.sh; comment compression

Findings

Blocking (must fix before merge):

  • scripts/ops/dev-cluster/migrate-dev-nodes.sh:59the plan response is echoed verbatim, and it carries MPC_ACCOUNT_SK. nomad_curl echoes the body for every non-GET method, and the plan call at line 128 asks for Diff: true. Nomad computes that diff contextually (oldJob.Diff(args.Job, true)), so unchanged fields come back with their values as Env[MPC_ACCOUNT_SK] field diffs — the nomad job plan CLI filters Type: "None" entries client-side, which is what -verbose re-enables. show_output's 1500-char cap is truncation, not redaction, and the key may land inside it. This contradicts RELEASES.md:186-187 ("Keys never appear on the command line or in the echoed output"). Echo only what the code consumes — you already extract FailedTGAllocs and Warnings at lines 131-132, so the plan call can opt out (a quiet flag on nomad_curl, or scrub Env/Template values with jq before show_output). Worth doing regardless of how a given Nomad version renders the diff: the body is arbitrary job content and the doc claim is unconditional.

  • scripts/ops/dev-cluster/migrate-dev-nodes.sh:167-170member signing keys are copied to the operator's disk before any confirmation, and never removed. ensure_job_keys runs for every discovered job ahead of the first confirm (line 136), so merely pointing the script at a cluster writes each member account's on-chain signing key into ~/.near-credentials/<net>/ permanently — including the mainnet dev cluster's *.dev-signer.near accounts. RELEASES.md:178-179 states "every write is behind a confirmation prompt", and dev-menu.sh:61's confirm "Proceed?" describes an image upgrade, not a key export; run standalone per the documented usage at line 7, there is no gate at all. Please (a) gate the import behind its own confirm naming what is written and where, and (b) prefer a run-scoped keystore removed on exit over a permanent ~/.near-credentials copy — a rollout does not need the keys to outlive it.

  • scripts/ops/prepare-github-release.sh:31 (with :73, :77) — pre-release versions are now accepted but only half-supported. check_version (common.sh:38) allows 3.14.0-rc1, but the OLD_VERSION awk pattern at line 73 and the sed bump at line 77 still require version = "X.Y.Z" with the closing quote right after the patch number. The first rc writes version = "3.14.0-rc1" into Cargo.toml, and every later run on that tree dies at line 74 with Could not find a workspace 'version = "X.Y.Z"' line — a misleading error for an input the script just accepted. Before this PR suffixes were rejected up front with a clear message. Either keep this script strict (local ^[0-9]+\.[0-9]+\.[0-9]+$ guard) or widen both patterns; .github/workflows/release.yml:34 already uses the suffix-tolerant awk form, so widening is the consistent choice.

  • scripts/ops/prepare-github-release.sh:81the compressed comment states the inverse of the code. abi_has_not_changed passes when the ABI is unchanged, which is why lines 83-84 die on success; the new comment says the test "fails if ABI was not affected". Suggest: # The bump must change the ABI snapshot, so this test is expected to fail — a pass means it didn't.

  • Doc-alignment items from the previous round are still open; per CLAUDE.md these block:

    • scripts/ops/menu.sh:25 and dev-menu.sh:63 still advertise a contract migration ("verify, then its contract", ### Step 1 — nodes) that nothing in scripts/ops/ implements. Drop the phrase and the numbering, or land them behind a TODO(#NNNN).
    • RELEASES.md:173-176 still ends the flow at the build-info check. dev-menu.sh:73 always reaches test_sign, which offers a real on-chain sign call with a 1 NEAR / 0.1 NEAR attached deposit from a member account (dev-common.sh:18-23, 122-131). Document the step, the deposit, the signing account, and MPC_SIGN_WITH.
    • RELEASES.md:179-180 "Nothing cluster-specific is stored in this repo" is contradicted by the hardcoded contract IDs and member accounts at dev-common.sh:18-23. Narrow it to addresses and credentials.

Non-blocking (nits, follow-ups, suggestions):

  • migrate-dev-nodes.sh:148require_cmds curl jq near omits python3, which import_signing_key hard-depends on; python3 is also absent from the dev shell (flake.nix:172-182), where the missing-dependency hint sends operators. A missing interpreter degrades to one warn per key and only surfaces later as an opaque near-cli signing failure.
  • first-time-setup.sh:73 — when a task injects MPC_ACCOUNT_SK via a template stanza rather than Env, || continue drops it silently; the operator finds out at test_sign. A warn for "account found, key not in Env" would close the loop.
  • dev-common.sh:122test_sign takes the signer from the hardcoded MEMBER_ACCOUNTS while the imported keys come from the job definitions; if they diverge the call fails opaquely. have_signing_key "$signer" already exists — check it and warn first.
  • first-time-setup.sh:14-30_B58.index(c) raises an unhandled ValueError (full traceback) on any non-base58 character, and a non-ed25519 prefix surfaces as "unexpected ed25519 key length". Both collapse into the same warn; a try/except with a one-line message would keep the output readable.
  • first-time-setup.sh:53-57 — the second keystore filename uses ed25519_<pubkey>.json. Worth confirming against near-cli-rs 0.25.1's legacy-keychain layout; if the separator is wrong the file is inert, i.e. an unread plaintext private key left on disk.
  • migrate-dev-nodes.sh:41--fail-with-body needs curl >= 7.76 and require_cmds only checks presence; this toolkit otherwise goes out of its way for BSD userland (prepare-github-release.sh:70).
  • prepare-github-release.sh:61-62 — the compressed git-cliff comment dropped the actual why: that a literal HEAD resolves to the default branch and so loses PRs merged only into a release branch. That is exactly the non-obvious constraint CLAUDE.md says to keep; what remains restates the flags. Same for the .cliffignore clause.
  • common.sh:43sha256_of still has no caller (carry-over).
  • Still no shellcheck gate for ~600 lines of Bash (carry-over). dev-common.sh:95 (for addr in ${MPC_NODE_ADDRS}) is intentional word-splitting and would need an explicit # shellcheck disable=SC2086 if one lands.

On "extremely difficult to test because e2e tests require connection to the real IP" — agreed for the rollout loop, but the pure parts need no cluster: image_in_job / job_with_image / job_signing_creds are jq filters over a fixture job JSON, curl_cfg_escape and fmt_cmd are string transforms, and the Python base58 round-trip takes a known-answer test. A few plain-Bash assertions over a checked-in fixture would cover what is most likely to break silently — especially job_with_image, where a wrong path expression registers a job that looks fine and starts the old image.

⚠️ Issues found

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.

1 participant