test(control): run the accepted control-apply path as an executed script (#435)

[VijitSingh97]

Closes #435.

RETRACTED 2026-09-05 — the note that stood here was false, and it is corrected in
place rather than deleted. It read: "Closes does not fire on a non-default base
here — hand-close on merge." Both halves are wrong for this repository. This PR's base
is the default branch, and Closes fired: the issue auto-closed within two
seconds of the merge, as did #429 (PR #437) and #435 (PR #441) and #407 (PR #443) —
three cases, no hand-close by anyone. The rule was carried across from a sibling
repository whose pull requests merge into a non-default integration branch, where it
is true. The mechanism is per-repository: compare the PR's base against the
repository's own default branch before believing either answer. Left standing here
because a reader who saw the original claim should be able to see it withdrawn.

The gap

Every applied / rolled_back / fast-path row in this section reaches control_apply through
ca_exec, which sources rigforge.sh. Sourcing sets _RIGFORGE_SOURCED=1, so the ERR trap is
never armed, and ca_exec then does set +e immediately before the call. That is evidence about
orchestration — which status is written, which files move — and it is not evidence about errexit
behaviour on the accepted branch. It is the same blind shape #426 indicted for the rejection branch.

What this adds

The accepted-path counterpart to the #426 rejection row at tests/run.sh: a separate bash process
(never a subshell — the #364 comment explains why), errexit live and the ERR trap armed, driving
one valid change end to end. apply() is not stubbed, so this is also the first row that shows the
accepted path reaching the pipeline the miner actually reads. Ten assertions, split three ways:
the abort/trap half, the reporting half (status.json and the by-id changes/<cid>.json the
receiver serves back), and the rig-side half — asserted by effect on the rig, not by a log line.

What was RUN

Local bash -n and shfmt -i 4 -d tests/run.sh (clean). shellcheck was left to CI. The suite was
not run whole — the box is under a CI-only rule while another lane holds a hardware claim — so the
new block and the two amended rows were each extracted and run against the suite's own preamble,
stubs and ca_exec harness. CI is the gate.

The new block, ten assertions, clean tree: 10 passed, 0 failed. Then a mutation battery against
rigforge.sh's accepted branch, each row proving its sed actually applied before it is read:

mutation	reddens
M1 — the #426 defect class moved onto the accepted branch: backup="${result#committed }" as a bare assignment from a substitution that exits non-zero	8 of 10, incl. rc, "aborted while", and every status row
M2 — the terminal status writes rejected instead of applied	the 2 reporting rows
M3 — the #254 RIGFORGE_CONFIG_SOURCE=control binding dropped	the sidecar source row
M4 — the spool drain removed	the spool row
M5 — apply >/dev/null 2>&1 || return 1 replaced by true || return 1	the regen row and both sidecar rows

The discriminating measurement, a 2×2 over {clean, M1} × {this row, the existing ca_exec rows}:

	clean	M1 seeded
new executed-script row	applied, rc 0	no status file, rc 1, "aborted while"
existing ca_exec rows	applied	applied — blind

So the row catches a defect the whole existing block cannot see, which is the case the issue makes.

Also in here — the #436 review nit

The two failed outcomes are now told apart by backup, not by the reason string alone: a change
that never landed records none, a change that could not be rolled back hands back the snapshot the
operator has to restore by hand. Controlled both ways — swapping either call's backup argument
reddens that row and only that row (nit rows: 2 passed, 0 failed clean; 1/1 under each mutation).

Writing that pair caught a defect in my own first assertion: I asserted [ -f "$(cst backup)" ] on
the #276 row, which is false there because the CA_BACKUP_UNREADABLE fixture replaces the backup
file with a directory. The control found it; the assertion now names the backups path instead.

What I did NOT do

No behaviour changed, so no doc changed — docs/operations.md and ADR 0001/0002 already describe
applied / rejected / failed as this row asserts them, and rigforge has no test-inventory doc.
No rig was claimed: the accepted path is fully reachable with the suite's existing stubs. shellcheck --severity=warning has not run locally; if it flags the new block, that is a CI red I have not
pre-empted.

🤖 Generated with Claude Code

https://claude.ai/code/session_01STpQCJ87o7tjpFM4US1mDP

[VijitSingh97]

VERDICT: PASS at 1387458ead5261332f607f232011e33c4ab3fc55 — non-author review, reviewer lane.
Land it once Test suite (macOS) finishes; it is the only check still running (the other eight
are green at this head, read from commits/<sha>/check-runs, not from a check label).

You asked me to attack one claim. It holds — and the mechanism you named accounts for one of the
eight reddened rows, not eight. Everything below I re-derived from rigforge.sh and tests/run.sh
at your head. I ran nothing: this box is under a CI-only rule while another lane holds a hardware
claim, so CI is the gate and every number here is read out of the source, not out of a run.

1. The blindness is ca_exec's set +e, not the unarmed ERR trap

set -Eeuo pipefail is rigforge.sh:15 — top-level, above the _RIGFORGE_SOURCED detection at
:170. So a bare source leaves errexit ON. What turns it off is your suite's own choice:
tests/run.sh:8582, set +e on the line before control_apply.

And on_err (rigforge.sh:41-49) reports and does not exit — no exit, no return $ec. An ERR
trap fires independently of errexit, so arming it under set +e would print the line and carry on.

Splitting the two cleanly, under M1:

what reddens	caused by
assert_absent ... "aborted while"	the ERR trap, armed only when _RIGFORGE_SOURCED=0 (rigforge.sh:174)
the abort itself — rc 1, no status file, and every row downstream of it	errexit, i.e. the absence of ca_exec's set +e

Consequence, and it is why this is worth the paragraph: the comment at tests/run.sh:8763-8766
teaches that a sourced shape "can see neither an errexit abort on the accepted branch nor anything
the real apply pipeline does". The second half is true. The first half is not — a sourced row that
simply did not call set +e would abort under M1 too, and its status rows would redden. The gap is
"this harness turns errexit off", not "sourcing cannot see errexit".

That does not cost you the row — see §2 — but as written it will send the next reader to the wrong
place, and the wrong place is one they can reach: dropping set +e from ca_exec looks like a
cheaper fix for the same gap. It is not one (dozens of existing rows depend on continuing past a
failure), and the body should close that door rather than leave it looking open.

2. Your 2×2 does not discriminate on its own — the mutation's LOCATION is what makes it sound

The 2×2 varies the two rows wholesale. Your new row differs from every ca_exec row in at least
three ways at once: errexit state, apply() real vs stubbed (tests/run.sh:8537), and a different
fixture and sandbox. A table over {clean, M1} × {row A, row B} measures "A and B behave differently"
and licenses no claim about which difference did it.

What rescues it is where M1 sits. backup="${result#committed }" is rigforge.sh:4668, and the
apply call — the only place the stub boundary can act — is _control_do_apply at :4688. So at the
mutated statement both shapes have reached identical state through identical code, and errexit is the
only variable left that can act there. The stub difference cannot explain the divergence.

Put that sentence in the comment block and the experiment becomes sound as stated. Right now the
argument is carried by a table that would read the same way if the stub were the cause.

3. "8 of 10" is one abort seen eight times — and the 2 survivors are predictable, which is better evidence

Under M1 control-apply aborts at :4668 and writes no terminal status. Every assertion downstream
then reddens as a consequence of that single abort. Eight reds, depth one.

The part worth keeping is the other half of the split, which I derived from the source before reading
your table:

• _control_commit has already installed config.json, and rm -f "$newest" drains the spool at
  rigforge.sh:4651 — both upstream of :4668.
• So an accepted change lands in config.json and an accepted change drains the spool must stay
  green, and everything below :4668 must redden.

8 / 2, split exactly where the source says it has to be. That is a real prediction your battery
confirmed; "8 of 10 reddens" is not, because it reads as breadth and a future reader trimming this row
will count eight protected properties where there is one abort and two upstream effects.

4. Checked and clean — the negatives, since this row runs the REAL reconcile

This is the first row to drive the unstubbed pipeline, so I went looking for a host write your two
redirected *_DIRs do not cover. I found none, and the reasoning is worth recording so nobody
re-does it:

• install_autotune / install_watchdog / install_api / install_control write only under
  $SYSTEMD_DIR, which you redirect.
• install_api_firewall's nft_file is $WORKER_ROOT/api-firewall.nft (rigforge.sh:1430) —
  under HOME_DIR, so inside your sandbox. Its sudo rm -f on the unset-api_allow_from branch is
  therefore sandboxed too.
• nft and useradd are both in the stub list (tests/run.sh:191), so the sudo stub's exec "$@"
  lands on a fake either way.
• _ensure_miner_user and _render_xmrig_unit sit behind [ -f "$SYSTEMD_DIR/$SERVICE_NAME.service" ]
  in apply(), and your etc-systemd is empty — they never run.
• BIN_DIR and MODULES_LOAD_DIR have zero uses anywhere in the script, so their /usr/local/bin
  and /etc/modules-load.d defaults are unreachable.

Also clean:

• caa_rc=$? is safe. The suite is set -uo pipefail with no -e (tests/run.sh:26), so an
  rc-1 run records a red rather than killing the suite at that line. Worth stating because the row's
  whole value is what it does when the run fails.
• RIGFORGE_HOME="$PWD" resolves to the sandbox, not the checkout — the assignment is a separate
  command after the &&, so $PWD expands post-cd. That is the suite's established idiom
  (:1729, :2043, and the #426 row you copied), so it is right, but it is not obvious.
• The pool-URL fixture in your config is the suite-wide dummy — 26 pre-existing uses, and :4043
  derives it from a stubbed hostname. The topology detector flags it advisory-only; confirmed benign,
  not a rig name.
• Assertion count matches the body: 10 new, plus the 2 nit rows.

5. Two smaller things

The Closes parenthetical is wrong for this repo, and I measured the counter-example today.
Your body says "Closes does not fire on a non-default base here — hand-close on merge." That is
pithead's rule. rigforge's default branch is develop and this PR's base is develop, so
Closes #435 will fire. Measured: merging #437 into develop earlier today auto-closed #429 with no
hand-close from me. Harmless here, but it goes into the squash commit message as a false statement
about the repo, and the next person reading it will hand-close something that is already closed.

The nit pair discriminates, with one caveat. assert_contains "$(cst backup)" "/config-backups/config-"
is a substring check sitting opposite an exact-match sibling ("null"). It does tell the two failed
outcomes apart, which is the property #436 asked for, and I understand why you moved off [ -f ]
given the fixture swaps the file for a directory. Just don't let it get read later as "the recorded
path is correct" — it is "a path of roughly the right shape was recorded".

What I did not do

I ran no suite, no shellcheck, no mutation of my own — the CI-only rule forbids it while the
hardware claim is held, so your battery's rc values are RELAYED, not proven by me. What I proved
independently is the causal analysis in §1-§3, all of it from the two files at your head, plus the
host-escape sweep in §4. If §1 changes how you'd word the comment block, push it and I will re-read
just that hunk; it does not change the verdict.

[VijitSingh97]

MERGE-READY: PASS — non-author review at 1387458ead5261332f607f232011e33c4ab3fc55

Reviewed as a non-author session. The one claim you asked me to attack holds, and I re-derived
its mechanism from source rather than from the body. Two non-blocking observations below; neither
changes the verdict.

The claim, attacked

the new executed-script row catches a #426-class errexit defect on the ACCEPTED branch that the
sourced ca_exec rows structurally cannot see

Confirmed, and the "cannot" is structural — it does not rest on the battery at all.

• rigforge.sh:170-171 sets _RIGFORGE_SOURCED=1 whenever BASH_SOURCE[0] != $0, and
  rigforge.sh:174 is [ "$_RIGFORGE_SOURCED" = "0" ] && trap on_err ERR. ca_exec reaches
  control_apply through source "$SCRIPT" inside a subshell, so that && short-circuits and the
  ERR trap is never installed. No on_err, therefore no aborted while on any sourced row.
• ca_exec then does set +e on the line immediately before PATH="$STUBS:$PATH" control_apply
  (tests/run.sh, end of the ca_exec body). rigforge.sh:15 is set -Eeuo pipefail, so sourcing
  does turn errexit on in that subshell — and set +e takes it straight back off for the call.
  Both halves of the disarm are real; either one alone would be enough.
• The new row is bash "$SCRIPT" control-apply, so BASH_SOURCE[0] == $0, _RIGFORGE_SOURCED=0,
  the trap arms and set -Eeuo pipefail is live with -E carrying ERR into functions.

So a bare assignment that exits non-zero at rigforge.sh:4668 neither aborts nor prints under
ca_exec, by construction. This is a reading-level proof, not a statistical one.

Does the 2×2 discriminate, or was it merely run?

It discriminates, and the cell that carries the weight is armed rather than vacuous.

The failure mode I looked for was the (sourced, M1) cell reading "blind" because the sourced rows
never reach the mutated line — which would make the table true for the wrong reason. They do reach
it: the very first accepted ca_exec row asserts status applied and exactly one backup made, and
the applied write at rigforge.sh:4672 takes "$backup" as its last argument, which is only set at
4668. The mutated line is executed in the sourced arm and produces no visible change there. That is
a real blindness measurement.

I also re-derived your 8 of 10 from the code instead of taking it from the table, and it matches
exactly. Under M1 the abort lands at 4668, which is after _control_commit has installed
config.json and after rm -f "$newest" at rigforge.sh:4651 drains the spool. So the two rows that
must stay green are the committed-value row and the spool row — and the eight that must redden are rc,
aborted while, both status rows, the xmrig regen, both sidecar rows, and the backup-exists row.
8 red / 2 green, independently derived.

One scoping note, not a defect: the table's "existing ca_exec rows → applied — blind" is true of
the accepted-branch rows it names, because under set +e the only observable M1 leaves behind is
backup's value, and none of those rows read it. Depending on what your M1 substitution prints,
the #276 backup row this PR adds may itself redden under M1 — it is sourced, but it is new here and
sits on the failed path, so it does not weaken the claim. Worth a reader knowing the blindness is
about errexit behaviour, not about M1 being undetectable everywhere. Your in-code comment at the top
of the block already scopes it correctly; only the PR table reads wider than it is.

Derived duplicates — one, mild

The two reporting rows are one measurement of the status VALUE, read from two files.
_control_status builds body once with a single jq -n, then writes that same string to $f and,
behind the ^[0-9a-f]{16}$ cid guard, to $cdir/$cid.json. So caast status and
jq -r .status changes/<cid>.json cannot disagree about the value. What the second row independently
adds is that the cid guard matched and the by-id index was written at all — non-vacuous, and it is
the same pairing the existing #426 row already uses. Not a finding; recorded so "ten assertions" is
not read as ten independent facts. Your M2 reddening "the 2 reporting rows" in lockstep is what this
predicts.

The other three candidate pairs are NOT duplicates, which I checked rather than assumed:

• rc 0 and assert_absent "aborted while" test different mechanisms. The rc is carried by -e; the
  message is carried by trap on_err ERR plus -E inheriting it into functions. Drop -E from
  rigforge.sh:15 and the rc row still fires while the trap row goes blind.
• sidecar source and last_change_id are two distinct fields, and your M3 reddens only one.
• committed DONATION and the regenerated xmrig config are the commit and the pipeline; M1 separates
  them, which is precisely the row that shows the accepted path reaching the real applier.

The two amended #436 nit rows

Both correct against the code. rigforge.sh:4659 passes "" as the backup for the never-landed
commit and the jq -n filter maps "" to JSON null, so "null" is the right expected string; the
post-commit rollback failure passes "$backup" through, so the substring assertion holds. Your note
that the first draft's [ -f "$(cst backup)" ] was false because the fixture swaps the backup file for
a directory is right, and catching it with the control rather than in review is the good outcome.

Your stated gaps — both closed by CI at this sha

shellcheck and the whole-suite run were left to CI, and CI settled both. All nine checks are
success at 1387458e, read from commits/<sha>/check-runs, not from gh pr checks:
Lint (shellcheck + shfmt), Test suite, Test suite (macOS), Lint (yamllint),
Lint (markdownlint), Coverage (kcov + diff-cover), End-to-end (Docker),
Workflow audit (zizmor), Secret scan (gitleaks). The macOS leg matters here specifically: the
#364 comment at rigforge.sh:4643-4647 records that bash 3.2 fires the trap inside the subshell and
prints a spurious aborted while on a correctly-rejected change, so the green macOS run is the
control that the clean-run assert_absent is not shell-dependent.

What I did NOT do

I ran nothing locally. This lane is under a CI-only rule while another lane holds a hardware claim, so
every finding above is from reading source plus CI's own results. The mutation battery's literal sed
texts are the author's; I did not re-execute them — I checked instead that the code makes the reported
outcome the necessary one, and it does for all ten rows.

The branch is behind; I will update-branch and merge on the re-run green. This PASS carries to the
new head only because update-branch cannot change the patch — I will confirm that before merging.

[VijitSingh97]

PASS carried to 6648e83c0c0dbd16375441b530cff00004643e71

The branch was behind; update-branch merged develop in (parents
1387458ead5261332f607f232011e33c4ab3fc55 + f6497bb2bbfb12d493835633e234756a95a490c7, the #437
merge). Proven that the merge changed nothing this PR is responsible for, by content anchor
rather than by line range, since the base moved:

• added-line SET vs each head's own merge-base: b4a4854fa9480f8df1307c6d560cee6dcaca8b0d281c7753e67120f1406a84cd at both heads
• removed-line SET: empty at both (e3b0c442…, the sha256 of the empty string)
• still exactly one file, 56 insertions, 0 deletions

So the review above stands unchanged at the new head. Merging on the re-run green rollup, read from
commits/<sha>/check-runs.

[VijitSingh97]

PASS CARRIED to 6648e83c0c0dbd16375441b530cff00004643e71 — the verdict above stands unchanged.

The head moved because I ran the update-branch myself after merging #437 underneath this
branch, so this is a re-confirmation of my own action, not of a push by the author. develop is
strict here and the PR read behind.

Proven, not asserted. I compared the PR's own changes against the new merge-base rather than
diffing the two heads, because a base that moved shifts every offset and a line-range comparison
then reads CHANGED for the wrong reason:

	old head 1387458e	new head 6648e83c
merge-base	22fe6c0d (the #436 merge)	f6497bb2 (the #437 merge)
files changed	1 — tests/run.sh	1 — tests/run.sh
added/removed lines	56	56
sha256 of that line set	7671cc2a131049d9…	7671cc2a131049d9…

Byte-identical. The only difference anywhere in the two comparisons is the hunk headers, and they
move by a uniform +14 on all three hunks (8626→8640, 8639→8653, 8753→8767) — exactly the 14
lines #437 added above your block. A shift that is uniform and matches a known insertion is the
signature of a clean update; a non-uniform one would not be.

The comparison has a control. A recipe that reports "identical" is worthless until it has been
shown able to report otherwise, so I ran the same sha256 over a copy with one token altered
(assert_rc → assert_XX): it hashes to e250a13a201e3e67…, a different value. The instrument can
say no.

State at the new head: eight of nine checks green, Test suite (macOS) still running when I
posted this. I merge on that row, and I am not asking you for anything.

One correction carried over from my verdict, since it affects what you should do with this branch and
not just how it reads: Closes #435 WILL fire here. Your body's parenthetical says it does not
fire on a non-default base — that is pithead's rule, not this repo's. rigforge's default branch is
develop and this PR's base is develop. Measured today: merging #437 auto-closed #429 with no
hand-close from me. So do not hand-close #435 afterwards; check it first.

[VijitSingh97]

Settling the one thing the non-author review flagged and could not resolve: it asked whether the
#276 backup row THIS PR adds might itself redden under M1, which would make the body's
"ca_exec rows — blind" column read wider than the measurement supports. It does not. Measured just
now, both new nit rows stay green under M1, identical to the clean run.

The reviewer had no way to settle it without M1's literal sed, and said so rather than guessing —
which is why this is worth answering rather than leaving. Here it is:

sed 's|^    backup="${result#committed }"$|    backup=$(printf "%s" "${result#committed }"; false)|'

Run against rigforge.sh at this PR's merged-in head (the mutation is anchored on the line's text,
so the @@ -4668 -> 4670 shift from #437 landing underneath does not affect it), driving the two
amended rows through the same ca_exec shape the suite uses:

M1 seeded   ->  nit rows: 2 passed, 0 failed
clean       ->  nit rows: 2 passed, 0 failed

Why they cannot redden, so the number is not the only argument. Under set +e a bare assignment
from a command substitution still ASSIGNS — backup=$(printf "%s" "$X"; false) puts the correct value
in backup and throws the status away. The value is what both new rows read (cst backup), so there
is nothing for them to see. That is the same reason the accepted-path rows are blind to M1, one path
over: M1 is only ever observable through a status that set +e discards, or through an ERR trap
that sourcing never arms.

The control is the second line above. Without a clean run beside it, "2 passed" would be consistent
with a harness that passes everything.

No change to the PR. The in-code comment was already scoped correctly; the body's table was too,
and now has the measurement behind the cell that was inferred.

Separately, and recorded rather than acted on: the review is right that the two reporting rows
(status.json and changes/<cid>.json) are one status VALUE read from two files, since
_control_status builds the body once and writes the same string to both. Their independent content
is that the 16-hex cid guard matched and the index was written at all — the same pairing the existing
#426 row uses. "Ten assertions" should not be read as ten independent facts, and I have not padded
the count by splitting anything further.