Skip to content

Commit e80e24e

Browse files
committed
fix(tooling): make inventory.sh's derived tag and lookup single-sourced
Follow-up to the input-validation pass, closing the rest of the review: - `validator` was special-cased in three places (a branch in hastag, plus two exemptions in the unknown-tag check) with `devnet-` hardcoded inside the branch, so a second derived tag meant touching all three and a fleet that names chains differently got a silent empty answer. It is now one table entry (prefix + disqualifying tag), and DEVNET_TAG_PREFIX names the prefix. - A literal `validator` in the file was silently ignored, which is exactly the disagreement the derivation exists to rule out: the row said one thing and the computation another. It now names the line and refuses. - `--tag validator` against a file carrying no chain tag at all said only "no host matches", when the actionable fact is that the tag can never match there. - The file-lookup ladder was a copy of devnet-env.sh's, comment included, and had already drifted (return-0 vs exit-2 on nothing found). Both now call one `devnet_find_file`, so the two config files of a deployment cannot end up with two ideas of where they live. - The known-tag list, whose whole job is to be read next to the operator's typo, printed in awk's hash order. Docs: devnet.inventory is the record of which hosts exist and `SERVERS` is a working set filled from a query against it, rather than two hand-kept host lists with no stated precedence. Also notes that the `for h in $SERVERS` workflows rely on bash word-splitting, which zsh does not do, and drops the em-dashes this branch added.
1 parent 245ea8d commit e80e24e

5 files changed

Lines changed: 126 additions & 44 deletions

File tree

.claude/skills/multi-server-devnet/SKILL.md

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,10 @@ B owns `k+1..N-1`). Everything below still applies with three changes, and
4040

4141
Nothing about the servers is hardcoded. Establish these from the operator:
4242

43-
- `SERVERS` — the SSH targets (any count, any names), one devnet each.
44-
- `SSH_USER` — login user. `docker` is invoked with `sudo`.
43+
- `SERVERS`: the SSH targets for the operation at hand (any count, any names),
44+
one devnet each. A working set taken FROM the inventory below, not a second
45+
list of hosts to keep in step with it.
46+
- `SSH_USER`: login user. `docker` is invoked with `sudo`.
4547
- Per-devnet `NODES` (validators on that server) and `SUBNETS`
4648
(`ATTESTATION_COMMITTEE_COUNT`); these can differ between servers.
4749
- A central host for Grafana + the federating Prometheus (often one of the
@@ -53,7 +55,7 @@ gitignored) instead of retyping them: the operator-side scripts source it via
5355
file. It is the one place a deployment's urls and Grafana ids live.
5456

5557
**The servers themselves live in `scripts/devnet.inventory`** (copy
56-
`scripts/devnet.inventory.example`; gitignored) name, ip, tags, and per-devnet
58+
`scripts/devnet.inventory.example`; gitignored): name, ip, tags, and per-devnet
5759
`NODES`/`SUBNETS`, one row per host. Query it with `scripts/inventory.sh` rather
5860
than reading it by hand:
5961

@@ -67,9 +69,10 @@ SERVERS=${SERVERS//$'\n'/ } # newlines -> spaces
6769

6870
Tag conventions: a `devnet-*` tag names the chain a host's nodes belong to (two
6971
hosts sharing one means the split-chain model, *not* two like-named devnets), and
70-
`aggregator` is a whole-server role. `validator` is **derived** — "has a `devnet-*`
71-
tag and is not tagged `aggregator`" — so it can never disagree with the aggregator
72-
tag. An unknown or empty `--tag` exits 2 rather than returning nothing (or, worse,
72+
`aggregator` is a whole-server role. `validator` is **derived**: "has a `devnet-*`
73+
tag and is not tagged `aggregator`", so it can never disagree with the aggregator
74+
tag, and writing it literally in the file is an error rather than a second opinion.
75+
An unknown or empty `--tag` exits 2 rather than returning nothing (or, worse,
7376
everything): a typo that yields an empty loop reports success while doing nothing,
7477
and `--tag "$UNSET"` would act on hosts the caller never named.
7578

@@ -151,7 +154,8 @@ redirect must run under sudo).
151154
## Workflows
152155

153156
Examples assume `SSH_USER` is set and you iterate over `SERVERS`. Per server you
154-
pass its own `NODES`/`SUBNETS`.
157+
pass its own `NODES`/`SUBNETS`. `for h in $SERVERS` splits under bash; zsh does not
158+
split parameter expansions, so there write `for h in $(echo $SERVERS)`.
155159

156160
### Pull the latest images on all servers
157161
```bash

.claude/skills/multi-server-devnet/scripts/devnet-env.sh

Lines changed: 27 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,9 @@
44
#
55
# . "$SCRIPT_DIR/devnet-env.sh"; devnet_load_env
66
#
7+
# Also provides devnet_find_file, the file-lookup ladder both this and
8+
# inventory.sh use.
9+
#
710
# Lookup order, first hit wins: $DEVNET_ENV, ./devnet.env, <scripts dir>/devnet.env.
811
# Copy devnet.env.example -> devnet.env and fill it in (devnet.env is gitignored,
912
# since it names your hosts and may point at a webhook file).
@@ -17,21 +20,32 @@
1720
# fine. A line that looks like an assignment but whose name isn't usable is
1821
# reported on stderr rather than dropped in silence, because a config line that
1922
# goes unread is how you deploy against the wrong deployment.
20-
devnet_load_env() {
21-
local dir file line key val
23+
24+
# Resolve one of the deployment's config files. First hit wins: the path in $2 (an
25+
# env var NAME), ./<name>, <scripts dir>/<name>. Prints the path on stdout.
26+
# Returns 1 when nothing exists, and 2 when the env var names a path that doesn't
27+
# -- an explicit path that isn't there is a typo, not a licence to fall back to
28+
# another deployment's file. Shared with inventory.sh so devnet.env and
29+
# devnet.inventory can't end up with two ideas of where they live.
30+
devnet_find_file() {
31+
local name=$1 envvar=$2 dir explicit candidate
2232
dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
23-
file=""
24-
if [ -n "${DEVNET_ENV:-}" ]; then
25-
# An explicit path that doesn't exist is a typo, not a reason to silently fall
26-
# back to some other devnet.env and deploy against the wrong deployment.
27-
[ -f "$DEVNET_ENV" ] || { echo "DEVNET_ENV=$DEVNET_ENV does not exist" >&2; return 1; }
28-
file=$DEVNET_ENV
29-
else
30-
for candidate in "./devnet.env" "$dir/devnet.env"; do
31-
[ -f "$candidate" ] && { file=$candidate; break; }
32-
done
33+
explicit=${!envvar:-}
34+
if [ -n "$explicit" ]; then
35+
[ -f "$explicit" ] || { echo "$envvar=$explicit does not exist" >&2; return 2; }
36+
printf '%s\n' "$explicit"; return 0
3337
fi
34-
[ -n "$file" ] || return 0
38+
for candidate in "./$name" "$dir/$name"; do
39+
[ -f "$candidate" ] && { printf '%s\n' "$candidate"; return 0; }
40+
done
41+
return 1
42+
}
43+
44+
devnet_load_env() {
45+
local file line key val rc
46+
file=$(devnet_find_file devnet.env DEVNET_ENV); rc=$?
47+
[ "$rc" -eq 2 ] && return 1 # DEVNET_ENV names a missing file: reported
48+
[ "$rc" -eq 0 ] || return 0 # no env file at all is not an error
3549

3650
while IFS= read -r line || [ -n "$line" ]; do
3751
line=${line#"${line%%[![:space:]]*}"} # ltrim, so an indented line is read

.claude/skills/multi-server-devnet/scripts/devnet.env.example

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,9 @@
55
# your shell overrides the value here.
66

77
# --- fleet (one independent devnet per server) -------------------------------
8+
# devnet.inventory is the record of which hosts exist; SERVERS is just the working
9+
# set for these scripts, so fill it FROM a query rather than maintaining a second
10+
# list: SERVERS=$(bash scripts/inventory.sh --tag devnet-ab --field name) || exit
811
SERVERS="host-a host-b" # ssh targets, any count/names
912
SSH_USER=user # login user; docker is invoked with sudo
1013

.claude/skills/multi-server-devnet/scripts/devnet.inventory.example

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
# SERVERS=$(bash scripts/inventory.sh --field name) || exit # exit 2 = typo'd tag
1616
# SERVERS=${SERVERS//$'\n'/ } # newlines -> spaces
1717
#
18-
# FORMAT whitespace-separated columns, aligned however you like. Blank lines and
18+
# FORMAT: whitespace-separated columns, aligned however you like. Blank lines and
1919
# lines whose first non-space character is '#' are ignored. Five columns:
2020
#
2121
# name ssh target / display name. Need not equal the machine's hostname,
@@ -28,7 +28,7 @@
2828
# nodes NODES on this server, or '-' if it runs no devnet.
2929
# subnets ATTESTATION_COMMITTEE_COUNT, or '-'.
3030
#
31-
# TAGS free-form, with two conventions inventory.sh knows about:
31+
# TAGS: free-form, with two conventions inventory.sh knows about:
3232
#
3333
# devnet-* names the chain this server's nodes belong to. Two servers sharing
3434
# one tag means they share one genesis, i.e. the split-chain model

.claude/skills/multi-server-devnet/scripts/inventory.sh

Lines changed: 83 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,10 @@
1515
# --count print how many rows matched, nothing else.
1616
#
1717
# `validator` is a DERIVED tag: "has a devnet-* tag and is not tagged aggregator".
18-
# It is computed, not read, so it can never disagree with the aggregator tag.
19-
# `--tag validator` is exactly `--tag <that devnet> --not-tag aggregator`.
18+
# It is computed from the row, never read from it, so it can never disagree with
19+
# the aggregator tag; writing it literally in the file is an error rather than a
20+
# second opinion. `--tag validator` is `--tag <that devnet> --not-tag aggregator`.
21+
# Set DEVNET_TAG_PREFIX if a fleet names its chains something other than devnet-*.
2022
#
2123
# An unknown `--tag` is an ERROR, not an empty result. A typo ('devnet5' for
2224
# 'devnet-5') that quietly returns no hosts turns `for h in $(inventory.sh ...)`
@@ -82,34 +84,60 @@ case $field in
8284
*) echo "--field $field: expected name|ip|tags|nodes|subnets|all" >&2; exit 2 ;;
8385
esac
8486

85-
# An explicit path that doesn't exist is a typo, not a reason to fall back to some
86-
# other inventory and act on the wrong fleet -- same rule devnet-env.sh applies.
8787
if [ -n "$file" ]; then
88+
# An explicit path that doesn't exist is a typo, not a reason to fall back to
89+
# some other inventory and act on the wrong fleet.
8890
[ -f "$file" ] || { echo "--file $file does not exist" >&2; exit 2; }
89-
elif [ -n "${DEVNET_INVENTORY:-}" ]; then
90-
file=$DEVNET_INVENTORY
91-
[ -f "$file" ] || { echo "DEVNET_INVENTORY=$file does not exist" >&2; exit 2; }
9291
else
92+
# Same ladder, from the same helper, that finds devnet.env: two config files of
93+
# one deployment should not have two ideas of where they live. Sourcing only
94+
# defines functions.
9395
dir="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
94-
for candidate in "./devnet.inventory" "$dir/devnet.inventory"; do
95-
[ -f "$candidate" ] && { file=$candidate; break; }
96-
done
97-
[ -n "$file" ] || {
96+
. "$dir/devnet-env.sh"
97+
file=$(devnet_find_file devnet.inventory DEVNET_INVENTORY); rc=$?
98+
# rc 2 means DEVNET_INVENTORY names a file that isn't there, already reported.
99+
[ "$rc" -eq 2 ] && exit 2
100+
[ "$rc" -eq 0 ] || {
98101
echo "no inventory file found (tried \$DEVNET_INVENTORY, ./devnet.inventory, $dir/devnet.inventory)" >&2
99102
echo "copy $dir/devnet.inventory.example to $dir/devnet.inventory and fill it in" >&2
100103
exit 2
101104
}
102105
fi
103106

104-
awk -v want="${tags[*]-}" -v nowant="${nottags[*]-}" \
107+
# Only the derived `validator` tag depends on how chains are named, so a fleet
108+
# that doesn't use devnet-* can say so instead of getting an empty answer.
109+
chain_prefix=${DEVNET_TAG_PREFIX:-devnet-}
110+
111+
awk -v want="${tags[*]-}" -v nowant="${nottags[*]-}" -v chain="$chain_prefix" \
105112
-v field="$field" -v docount="$count" -v src="$file" '
106-
# Tags are matched against ",a,b," so "agg" never matches "aggregator".
113+
# Tags are matched against ",a,b," so "agg" never matches "aggregator". A
114+
# DERIVED tag is computed from the row instead: it carries some chain tag and
115+
# is not disqualified by a role tag.
107116
function hastag(t, x) {
108-
if (x == "validator")
109-
return (index(t, ",devnet-") > 0 && index(t, ",aggregator,") == 0)
117+
if (x in DPREFIX)
118+
return (index(t, "," DPREFIX[x]) > 0 && !hastag(t, DEXCLUDE[x]))
110119
return index(t, "," x ",") > 0
111120
}
112-
BEGIN { nw = split(want, W, " "); nn = split(nowant, NW, " ") }
121+
122+
# `for (k in a)` walks a hash order, and these lists get read by a human next to
123+
# their own typo. Insertion sort: the tag list is a few dozen entries at most.
124+
function sortkeys(a, out, i, j, n, v) {
125+
n = 0
126+
for (i in a) out[++n] = i
127+
for (i = 2; i <= n; i++) {
128+
v = out[i]
129+
for (j = i - 1; j >= 1 && out[j] > v; j--) out[j+1] = out[j]
130+
out[j+1] = v
131+
}
132+
return n
133+
}
134+
135+
BEGIN {
136+
nw = split(want, W, " "); nn = split(nowant, NW, " ")
137+
# One entry per derived tag: the tag PREFIX a row must carry, and the tag that
138+
# disqualifies it. Adding a derived tag is a line here, not a new branch.
139+
DPREFIX["validator"] = chain; DEXCLUDE["validator"] = "aggregator"
140+
}
113141
114142
{ sub(/^[[:space:]]+/, "") }
115143
/^#/ || /^$/ { next }
@@ -137,9 +165,14 @@ awk -v want="${tags[*]-}" -v nowant="${nottags[*]-}" \
137165
}
138166
t = "," tags ","
139167
140-
# Every literal tag in the file, so a typo can be told from a real absence.
168+
# Every literal tag in the file, so a typo can be told from a real absence,
169+
# and where it was first written, so both can be pointed at a line.
141170
n = split(tags, TT, ",")
142-
for (i = 1; i <= n; i++) if (TT[i] != "") seen[TT[i]] = 1
171+
for (i = 1; i <= n; i++) if (TT[i] != "") {
172+
if (!(TT[i] in seen)) seenrow[TT[i]] = FNR
173+
seen[TT[i]] = 1
174+
if (substr(TT[i], 1, length(chain)) == chain) chainseen = 1
175+
}
143176
144177
for (i = 1; i <= nw; i++) if (!hastag(t, W[i])) next
145178
for (i = 1; i <= nn; i++) if ( hastag(t, NW[i])) next
@@ -161,14 +194,42 @@ awk -v want="${tags[*]-}" -v nowant="${nottags[*]-}" \
161194
}
162195
163196
bad = 0
164-
for (i = 1; i <= nw; i++)
165-
if (W[i] != "validator" && !(W[i] in seen)) {
197+
198+
# A derived tag written as a literal is the exact disagreement the derivation
199+
# exists to rule out: the row would claim one thing and the computation
200+
# another. Say which line, rather than ignoring what the operator wrote.
201+
nd = sortkeys(DPREFIX, D)
202+
for (i = 1; i <= nd; i++)
203+
if (D[i] in seen) {
204+
printf "%s:%d: '\''%s'\'' is a DERIVED tag (%s* and not %s), remove it from the file\n", \
205+
src, seenrow[D[i]], D[i], DPREFIX[D[i]], DEXCLUDE[D[i]] > "/dev/stderr"
206+
bad = 1
207+
}
208+
209+
for (i = 1; i <= nw; i++) {
210+
if (W[i] in DPREFIX) {
211+
# Derived, so never "unknown" -- but if no row carries a chain tag it can
212+
# never match, and that is a stale inventory, not a query to answer.
213+
if (!chainseen) {
214+
printf "no %s* tag in %s, so %s matches nothing (set DEVNET_TAG_PREFIX?)\n", \
215+
chain, src, W[i] > "/dev/stderr"
216+
bad = 1
217+
}
218+
continue
219+
}
220+
if (!(W[i] in seen)) {
166221
printf "no such tag in %s: %s\n", src, W[i] > "/dev/stderr"; bad = 1
167222
}
223+
}
224+
168225
if (bad) {
169226
printf "known tags: " > "/dev/stderr"
170-
for (k in seen) printf "%s ", k > "/dev/stderr"
171-
printf "(+ derived: validator)\n" > "/dev/stderr"
227+
nk = sortkeys(seen, K)
228+
# A derived name that leaked into the file is listed as derived, not twice.
229+
for (i = 1; i <= nk; i++) if (!(K[i] in DPREFIX)) printf "%s ", K[i] > "/dev/stderr"
230+
printf "(+ derived:" > "/dev/stderr"
231+
for (i = 1; i <= nd; i++) printf " %s", D[i] > "/dev/stderr"
232+
printf ")\n" > "/dev/stderr"
172233
exit 2
173234
}
174235

0 commit comments

Comments
 (0)