chaos: install harness configs atomically, and make the harness shellcheck-clean

[allamiro]

Stacked on #252 → #251 → #250. Bases retarget automatically as each merges.

Scenario 01 failed intermittently with:

error: parse .../admin-1.yaml: missing field `endpoint`

An error about the harness's own scratch file, which reads like a product fault and points nowhere near its cause.

The race

Config emitters are called per invocation, not once — meta_admin re-emits its client config on every call. Scenario 01 runs a proposal loop in the background while the foreground drives membership changes:

propose_loop &                                    # calls meta_admin in a loop
...
meta_admin "$LEADER_ID" add-learner --node-id 4   # and again here

Two writers re-emit the same path concurrently, and > "$cfg" truncates in place — leaving a window where a reader parses a fragment. All 12 emitters now write to a temp file and rename, which is atomic within a directory: a reader sees either the previous complete config or the new one, never a partial.

This was always latent; the same hazard existed under the old filename. It surfaced as a scenario that passed or failed depending on scheduling — the worst failure mode for a suite about to gate CI, because it teaches people to re-run instead of read.

Shellcheck-clean

Also silences the harness's one remaining finding with its reason rather than a blanket exclusion: the single quotes around the unshare probe are deliberate, because $1 must be expanded by the inner bash -c, not the outer shell.

The tree is now clean under:

shellcheck -x -P scripts/live-chaos/scenarios scripts/live-chaos/**/*.sh

-P matters — without it, source "$(dirname …)/../lib.sh" can't be resolved and every scenario reports SC1091 plus spurious SC2153 "may not be assigned" warnings for variables lib.sh does define. That is what lets the next change lint this tree in CI without drowning it in false positives.

Verification

Scenario 01 passes. Full-suite re-run in progress; the three scenarios fixed across this stack (09, 10, 11) all pass, and the remaining local failures are the four that need Linux (unshare for 05/05b/06, a clock shim for 07) and are expected to run on CI's ubuntu runners.

---

Summary by cubic

Install atomic config writes in the chaos harness to prevent partial YAML reads during concurrent emits, and clean up the last shellcheck warning. This removes the flaky Scenario 01 error and makes the harness ready for CI linting.

• Bug Fixes
  • Add install_config to write via temp file + rename, avoiding truncation races (no more intermittent “missing field endpoint”).
  • Silence shellcheck in require_mount_namespace with a targeted SC2016 disable around the unshare probe so $1 expands in the inner bash -c.

^{Written for commit e19ecef. Summary will update on new commits.}

[cursor]

Bugbot is not enabled for your account, so this pull request was not reviewed.

Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs.

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

[cubic-dev-ai]

4 issues found across 1 file

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="scripts/live-chaos/lib.sh">

<violation number="1" location="scripts/live-chaos/lib.sh:318">
P2: Using `mktemp` here adds an unconditional dependency that the documented exact `CHAOS_WORKDIR` mode does not preflight. On a minimal supported host without `mktemp`, the run reaches config emission and fails only then; preflight `mktemp` for both modes or provide a supported fallback.</violation>

<violation number="2" location="scripts/live-chaos/lib.sh:318">
P3: Every emitted config file now silently changes permissions from the shell's default umask (typically 0644) to 0600, because `mktemp` creates the temp file with mode 0600 and `mv` preserves that mode. Today that is harmless — the harness launches vtop-node as the same user — but it is an unannounced regression across all 12 emitters, and it will break any future consumer that reads these configs as a different user (e.g. a container mount, sudo-run node, or a shared workdir). Consider pinning the final mode so the change in behavior is explicit and stable.</violation>

<violation number="3" location="scripts/live-chaos/lib.sh:319">
P1: A failed config write can still replace the live config with a partial file because `cat`'s status is ignored and `mv` always runs. On ENOSPC or another I/O error, readers get an atomically renamed but truncated YAML instead of the previous complete config; remove the temp file and return before renaming when `cat` fails.</violation>

<violation number="4" location="scripts/live-chaos/lib.sh:354">
P2: An installation failure is hidden from callers because every emitter prints `$cfg` after the pipeline regardless of its status, so command-substitution callers can start nodes or CLIs with a missing or stale config. Return the pipeline failure before printing the path (for example, `} | install_config "$cfg" || return 1`) in each emitter.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[cubic-dev-ai]

P1: A failed config write can still replace the live config with a partial file because cat's status is ignored and mv always runs. On ENOSPC or another I/O error, readers get an atomically renamed but truncated YAML instead of the previous complete config; remove the temp file and return before renaming when cat fails.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At scripts/live-chaos/lib.sh, line 319:

<comment>A failed config write can still replace the live config with a partial file because `cat`'s status is ignored and `mv` always runs. On ENOSPC or another I/O error, readers get an atomically renamed but truncated YAML instead of the previous complete config; remove the temp file and return before renaming when `cat` fails.</comment>

<file context>
@@ -296,6 +301,25 @@ require_binaries() {
+install_config() {
+  local path="$1" tmp
+  tmp="$(mktemp "$path.XXXXXX")" || return 1
+  cat > "$tmp"
+  mv -f "$tmp" "$path"
+}
</file context>

Suggested change

	cat > "$tmp"
	mv -f "$tmp" "$path"
	if ! cat > "$tmp"; then
	rm -f "$tmp"
	return 1
	fi
	mv -f "$tmp" "$path"

[cubic-dev-ai]

P2: Using mktemp here adds an unconditional dependency that the documented exact CHAOS_WORKDIR mode does not preflight. On a minimal supported host without mktemp, the run reaches config emission and fails only then; preflight mktemp for both modes or provide a supported fallback.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At scripts/live-chaos/lib.sh, line 318:

<comment>Using `mktemp` here adds an unconditional dependency that the documented exact `CHAOS_WORKDIR` mode does not preflight. On a minimal supported host without `mktemp`, the run reaches config emission and fails only then; preflight `mktemp` for both modes or provide a supported fallback.</comment>

<file context>
@@ -296,6 +301,25 @@ require_binaries() {
+# either the previous complete config or the new one, never a fragment.
+install_config() {
+  local path="$1" tmp
+  tmp="$(mktemp "$path.XXXXXX")" || return 1
+  cat > "$tmp"
+  mv -f "$tmp" "$path"
</file context>

[cubic-dev-ai]

P2: An installation failure is hidden from callers because every emitter prints $cfg after the pipeline regardless of its status, so command-substitution callers can start nodes or CLIs with a missing or stale config. Return the pipeline failure before printing the path (for example, } | install_config "$cfg" || return 1) in each emitter.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At scripts/live-chaos/lib.sh, line 354:

<comment>An installation failure is hidden from callers because every emitter prints `$cfg` after the pipeline regardless of its status, so command-substitution callers can start nodes or CLIs with a missing or stale config. Return the pipeline failure before printing the path (for example, `} | install_config "$cfg" || return 1`) in each emitter.</comment>

<file context>
@@ -327,7 +351,7 @@ emit_meta_config() {
     fi
     echo "observability: { listen: \"$(meta_metrics_addr "$id")\" }"
-  } > "$cfg"
+  } | install_config "$cfg"
   echo "$cfg"
 }
</file context>

[cubic-dev-ai]

P3: Every emitted config file now silently changes permissions from the shell's default umask (typically 0644) to 0600, because mktemp creates the temp file with mode 0600 and mv preserves that mode. Today that is harmless — the harness launches vtop-node as the same user — but it is an unannounced regression across all 12 emitters, and it will break any future consumer that reads these configs as a different user (e.g. a container mount, sudo-run node, or a shared workdir). Consider pinning the final mode so the change in behavior is explicit and stable.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At scripts/live-chaos/lib.sh, line 318:

<comment>Every emitted config file now silently changes permissions from the shell's default umask (typically 0644) to 0600, because `mktemp` creates the temp file with mode 0600 and `mv` preserves that mode. Today that is harmless — the harness launches vtop-node as the same user — but it is an unannounced regression across all 12 emitters, and it will break any future consumer that reads these configs as a different user (e.g. a container mount, sudo-run node, or a shared workdir). Consider pinning the final mode so the change in behavior is explicit and stable.</comment>

<file context>
@@ -296,6 +301,25 @@ require_binaries() {
+# either the previous complete config or the new one, never a fragment.
+install_config() {
+  local path="$1" tmp
+  tmp="$(mktemp "$path.XXXXXX")" || return 1
+  cat > "$tmp"
+  mv -f "$tmp" "$path"
</file context>