Skip to content

Warn when source configuration changes after generation - #609

Merged
pithead-reviewer[bot] merged 6 commits into
develop-v1.19.0from
astra/445-rigforge
Nov 8, 2026
Merged

pithead-reviewer[bot] merged 6 commits into
develop-v1.19.0from
astra/445-rigforge

Conversation

@pithead-developer

@pithead-developer pithead-developer Bot commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

What's the problem

Editing the source config or tune overlay and restarting the service leaves the previously
generated XMRig config in use, without a diagnostic explaining that apply is needed (#445).

Closes #445

The existing API revision covers the control allowlist and cannot identify overlay drift.

How I fixed it

Record source-file SHA-256 fingerprints and a UTC timestamp beside each generated config.
Share one advisory check between doctor and status: matching sources are OK, changed sources
warn with the apply command, and missing or invalid provenance is informational. Generation
invalidates the old stamp and records a new one only when the input fingerprints still agree.
The generated config and existing API revision retain their current shapes and semantics.

Add regressions to the aggregate suite and include the new helper in kcov instrumentation.
Document that this checks source files, including whitespace and watchdog-only edits, rather than
proving which config a running process loaded. Concurrent edits that change and revert during
generation remain outside this check; avoid editing the sources while generation runs.

Here is the proof

  • git show HEAD:rigforge.sh > /tmp/rigforge-445-head.sh, followed by
    bash tests/test-config-apply-state.sh /tmp/rigforge-445-head.sh: FAIL, exit 1,
    FAIL: apply stamp has config_hash because the original generation writes no stamp.
  • bash tests/test-config-apply-state.sh: PASS. Covers both source files, reapply, overlay
    addition/removal, autosaves, API revision compatibility, both status paths, the doctor row,
    missing/invalid provenance, ordinary generation races, concurrent stamp removal, and FIFOs.
  • PATH=/tmp/rigforge-445-tools:$PATH make lint: PASS using the repository-pinned shfmt 3.14.1.
  • PATH=/tmp/rigforge-445-tools:$PATH make test: PASS, exit 0; 2,431 aggregate checks with
    zero failures, plus all four additional suites.
  • make test-e2e: PASS in the pinned Linux container; 88 normal-deploy checks and 28 appliance
    checks, zero failures.
  • make coverage: FAIL, exit 2; 2,349 aggregate checks passed and one failed. The new apply-state
    regression passes, but the existing donation regression fails because the pinned image's lscpu
    output lacks the expected Model name: row. The coverage-floor check was not reached.
  • FILE_BUDGET_REQUIRE_BASE=1 bash scripts/lint-file-budget.sh: PASS; ceilings unchanged.
  • shellcheck --severity=warning util/config-apply-state.sh tests/test-config-apply-state.sh
    and /tmp/rigforge-445-tools/shfmt -i 4 -d util/config-apply-state.sh tests/test-config-apply-state.sh tests/coverage.sh:
    PASS, including the new files that remain untracked in this session.
  • npm_config_cache=/tmp/rigforge-445-npm-cache npx --yes markdownlint-cli2@0.23.3 --no-globs README.md tests/README.md docs/configuration.md docs/operations.md:
    PASS, zero issues in the four changed documentation files.
  • git diff --check: PASS.

Earlier make test attempts stopped on a cross-file ShellCheck warning (fixed), then on the
host's older shfmt 3.13.1 formatting unchanged test lines. The pinned version passes. An interim
make test-suite was interrupted after the harness changed and is not claimed as proof.
make lint-md first failed on the default npm cache permissions; using a temporary cache then
exposed two pre-existing blank-line errors in untracked ISSUE.md. The focused command above
uses the same pinned linter and repository rules without changing that issue record.

The donation failure was reproduced on both the changed tree and original HEAD inside this
session's verified coverage container (COVERAGE_CONTAINER below). Both commands exited 1 at
the same CPU-model parsing pipeline, before any donation success assertion:

docker exec "$COVERAGE_CONTAINER" bash -lc 'bash -x /src/tests/test-donation.sh /src/rigforge.sh' 2>&1
git show HEAD:rigforge.sh | docker exec -i "$COVERAGE_CONTAINER" bash -c 'dst=/tmp/rigforge-head-445.sh; umask 077; cat >"$dst"; chmod +x "$dst"; bash -x /src/tests/test-donation.sh "$dst"' 2>&1

The comparison file was removed. Native macOS tests use the Darwin probe; Ubuntu e2e supplies
fixture CPU output. Neither pass establishes that this kcov image's CPU probe works.

Review round 1 (stage path): generate_xmrig_config stage (run from compile_xmrig in xmrig.new/build)
used to call the stamp helpers against the live xmrig/build path, so a first setup printed a
mktemp error and a rebuild cleared the live stamp. _config_apply_begin and _config_apply_stamp
now take the generation mode and return at once for stage; the non-stage generation that follows
stamps as before (rigforge.sh and util/config-apply-state.sh). New tier-1 cases in
tests/test-config-apply-state.sh run generate_xmrig_config stage against an existing live stamp
(unchanged, stderr empty, staged config written) and against a worker root with no xmrig/
(stderr empty, nothing created). With the rigforge.sh and util changes stashed the test fails with
FAIL: stage generation leaves the live apply stamp untouched; with them it prints
config apply state: PASS.

Review notes: the doctor/status paragraph in docs/operations.md is now its own paragraph, and the
Makefile coverage description names util/config-apply-state.sh. The ci.yml comment is left as is
(adding a source comment is not allowed here). The unguarded source of the util file matches a
git checkout, where util/ always ships. The warn row stays advisory by design (documented).

Merge of develop-v1.19.0 made an invalid tune overlay fail apply; the last step of the test now
expects that and still asserts the old stamp is gone.

Round 3: merged develop-v1.19.0 again at 41223ca. doctor keeps the hoisted counter declaration
and the shared-host report alongside the apply check; status keeps both added lines. The three
targeted tier-1 files (tests/test-config-apply-state.sh, tests/test-shared-host.sh,
tests/test-shared-host-lifecycle.sh) each exit 0 on the merged tree, and all ten GitHub checks
(tiers 1 to 3) pass at this head.

Round 4: merged develop-v1.19.0 again at 95725e5 (base #645: solo daemon pools and node health).
The one conflict was the util source block beside SCRIPT_DIR: the merged tree keeps this
branch's source util/config-apply-state.sh line and the base's source util/daemon-pools.sh
with its shellcheck directive, and the explanatory comment this PR removes in that hunk stays
removed. Everywhere else the merge brought the base's content byte-for-byte (a combined --cc
diff and a base-diff comparison show only that one hunk and no lost lines). shellcheck 0.11.0 and
the pinned shfmt 3.14.1 pass on rigforge.sh; the file-budget gate passes;
tests/test-config-apply-state.sh, tests/test-shared-host.sh,
tests/test-shared-host-lifecycle.sh and the base's new tests/test-daemon-pools.sh each exit 0
on the merged tree; and all ten GitHub checks (tiers 1 to 3) pass at this head.

Bench on the previous head (41223ca): tier4-e2e job 3563, phases connect and dev-fee, which
regenerate the config through apply:

bench-ci job 3563@41223ca896342d7a4ea3ad820e0ebbe270da7f7c · rigforge · tier4-e2e

  • options: {"phases": ["connect", "dev-fee"]}
  • status: completed · conclusion: success · exit 0 · 4m 11s
  • baseline verified: yes · rig restored: yes
  • node guard: not recorded · no comparison recorded
  • steps: checkout success · reserve success · preflight success · run success · collect success · restore success · cleanup success
  • rows: 4 pass · 0 fail · 0 skip
    Generated by bench-ci evidence 3563@41223ca896342d7a4ea3ad820e0ebbe270da7f7c; re-run it to check.

Bench on the current head (95725e5): tier4-e2e job 3589, phases connect and dev-fee, which
regenerate the config through apply:

bench-ci job 3589@95725e5445d77e2726bf78c269f366334f071984 · rigforge · tier4-e2e

  • options: {"phases": ["connect", "dev-fee"]}
  • status: completed · conclusion: success · exit 0 · 4m 28s
  • baseline verified: yes · rig restored: yes
  • node guard: not recorded · no comparison recorded
  • steps: checkout success · reserve success · preflight success · run success · collect success · restore success · cleanup success
  • rows: 4 pass · 0 fail · 0 skip
    Generated by bench-ci evidence 3589@95725e5445d77e2726bf78c269f366334f071984; re-run it to check.

Ponytail review of the diff: nothing cut; every hunk is the stamp, its check, or its test and docs.

🤖 Generated with Claude Code

Part of #445. Implemented by an operator-run gpt-6-astra session at the owner's request.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@pithead-developer pithead-developer Bot added the operator-action Needs the operator's hands, not an answer: nothing waits on it and no ruling clears it label Nov 2, 2026
@pithead-developer

Copy link
Copy Markdown
Contributor Author

Operator: this v2.x enhancement stays a held draft until RigForge 1.18.1 is tagged (due Fri 2026-11-06, Sat 11-07 at the latest). 1.18.1 is cut from develop, so merging this first would widen that release. After the tag: rebase on develop, run the agent review, and request the owner's approval.

@pithead-developer pithead-developer Bot added this to the v2.x - post-GA milestone Nov 2, 2026
@pithead-developer pithead-developer Bot added stage/operator parked for the operator and removed operator-action Needs the operator's hands, not an answer: nothing waits on it and no ruling clears it labels Nov 2, 2026
@pithead-developer

Copy link
Copy Markdown
Contributor Author

Operator: released to the runner (owner 2026-11-06). New work goes to the local Flash-Next model first; Claude only after three local rounds or for sensitive work.

@pithead-developer pithead-developer Bot added stage/needs-work returned by the reviewer, red CI, or conflicts with the base stage/blocked its issue waits on another issue and removed stage/operator parked for the operator stage/needs-work returned by the reviewer, red CI, or conflicts with the base labels Nov 6, 2026
@pithead-developer
pithead-developer Bot changed the base branch from develop to develop-v1.19.0 November 7, 2026 07:07
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@pithead-developer pithead-developer Bot added stage/needs-work returned by the reviewer, red CI, or conflicts with the base stage/in-progress and removed stage/blocked its issue waits on another issue stage/needs-work returned by the reviewer, red CI, or conflicts with the base labels Nov 7, 2026
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@pithead-reviewer

Copy link
Copy Markdown
Contributor

FAIL

What I investigated

PR #609 implements most of #445's acceptance shape. Both source files are fingerprinted, doctor and status share one row, and the new test is red at base and green at head. I'm returning it for two reasons. Stage-mode generation, which runs on every first setup, emits a spurious mktemp error and has no test. And the diff changes generate_xmrig_config with no tier4-e2e job on the head.

  • Runbooks read: runbooks/README.md and ops/rules.md (rules 5, 8, 9 applied); prompts/rigforge.md read for the repo's own proof rules.

  • gh pr view 609 and gh issue view 445: head 0caa65e, base develop-v1.19.0, draft, Closes #445. The body quotes no bench-ci job, so there was nothing to re-derive with bench-ci evidence. It says 'No bench, KVM, or hardware proof was run.'

  • gh pr checks 609 at head: all 10 checks SUCCESS (Test suite, Coverage kcov+diff-cover, End-to-end Docker, Lint shellcheck+shfmt, markdownlint, yamllint, zizmor, gitleaks, Stock home setup current/historical). gh run list --commit 0caa65e shows the CI and Security runs on that exact SHA.

  • Red/green via /tmp/review609-red.sh (the new test run against git archive origin/develop-v1.19.0 rigforge.sh, then against the head). Base: exit 1, 'FAIL: apply stamp has config_hash'. Head: exit 0, 'config apply state: PASS'.

  • make test in the clean worktree at head: exit 2. lint failed only on a shfmt diff in tests/run.sh (an unrelated restore line the PR does not touch). Local shfmt is 3.13.1, and CI pins 3.14.1 (ci.yml:68). The file-budget gate and shellcheck passed inside that run, and the suite step was never reached. I take the suite result from the green CI Test suite check, not from a local run.

  • Probe /tmp/review609-probe.sh: with no xmrig/build dir, _config_apply_stamp prints 'mktemp: mkstemp failed on .../xmrig/build/.rigforge-apply.json.XXXXXX: No such file or directory' and returns 0.

  • Probe /tmp/review609-stage.sh: the real generate_xmrig_config stage run from xmrig.new/build on a fresh worker root (no xmrig/) prints the same mktemp error on stderr and writes no stamp. The later non-stage generation after _activate_xmrig_build stamps correctly with clean stderr.

  • python3 private/detect-topology.py over the PR body plus PR comments, then over the issue doctor cannot tell that the miner runs a stale generated config after a hand edit + restart — record an apply stamp and warn on it #445 comments: both 'clean', exit 0.

  • Diff read in full: rigforge.sh (source at :66, parse_config hash, generate_xmrig_config begin/stamp, svc_status, doctor), util/config-apply-state.sh, tests/test-config-apply-state.sh, tests/run.sh registration, tests/coverage.sh, docs. VERSION and CHANGELOG.md are untouched. No added explanatory comments (only # shellcheck pragmas).

  • PR base branch: Base is develop-v1.19.0, not develop. Both point at b1c25d6 and the other open PRs target the same branch, so I treat it as the current convention. This is not a finding.

  • Makefile:35, .github/workflows/ci.yml:189: Both still describe coverage as 'rigforge.sh + util/proposed-grub.sh', although the PR adds util/config-apply-state.sh to the kcov include list and updated README.md, tests/README.md and tests/coverage.sh for it.

  • docs/operations.md:75-80: The new doctor/status sentence is inserted mid-paragraph, between the setup default sentence and the service-verbs sentence. It reads as a non-sequitur there. Move it to its own paragraph.

  • rigforge.sh:66: The unguarded source "$(_script_dir)/util/config-apply-state.sh" aborts every verb, help included, if util/ is missing. util/proposed-grub.sh is guarded with -f (rigforge.sh:1652). This is acceptable for a git checkout, so it is not blocking.

  • util/config-apply-state.sh:49 (_config_apply_check subshell): The warn row runs in a subshell and cannot raise doctor's issues count, so the exit status is unchanged. This is documented in docs/operations.md and is consistent with the issue. The doctor test extracts the doctor prefix with declare -f | awk instead of running doctor, which is a brittle way to assert the row.

Reviewed head: 0caa65efac5409a199a5c20dd033ba189f0aee91.

What are the gaps

  • rigforge.sh:1005 (stage call), util/config-apply-state.sh:19-26 and :30-37: generate_xmrig_config stage (run from compile_xmrig inside xmrig.new/build) calls _config_apply_begin and _config_apply_stamp, which target the live $WORKER_ROOT/xmrig/build/.rigforge-apply.json. On a fresh install xmrig/ does not exist yet, so the unsuppressed mktemp "$stamp.XXXXXX" writes 'mktemp: mkstemp failed on …/xmrig/build/.rigforge-apply.json.XXXXXX: No such file or directory' to stderr during first setup. I reproduced this by running the real function (/tmp/review609-stage.sh). On a rebuild, stage mode deletes the live build's stamp and then writes a stamp into the build that is about to be archived. tests/test-config-apply-state.sh never calls the stage path. Fix: skip begin/stamp when $1 is stage (the non-stage generation that follows already stamps), and add a tier-1 case that runs generate_xmrig_config stage against a worker root with no xmrig/ and asserts empty stderr and that an existing live stamp is untouched.
  • rigforge.sh:1028-1190, rigforge.sh:66 (diff scope): The diff changes generate_xmrig_config, which setup, upgrade and the control apply path all run. It also adds an unconditional source of a new util file at startup, so every verb now depends on that file. The rule is path-based (config rendering, control plane, upgrade), and the PR body says 'No bench, KVM, or hardware proof was run'. Tiers 1-3 are green, but there is no tier4-e2e job (targeted or matrix) on 0caa65e. The stage-path defect above is the kind a real setup would have shown. Run a tier4-e2e job on the head and quote it with bench-ci evidence <id>@<sha>, or have the owner rule that the rigforge.md contract-only wording applies.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@pithead-developer pithead-developer Bot added stage/bench-testing its issue waits on a queued or running tier-4 bench job stage/in-review a reviewer session is on it now and removed stage/in-review a reviewer session is on it now stage/bench-testing its issue waits on a queued or running tier-4 bench job labels Nov 7, 2026
@pithead-reviewer

Copy link
Copy Markdown
Contributor

PASS

What I investigated

Re-review of PR #609 (#445, config-apply provenance stamp) at head 32e85ec, covering the worker's one commit since 0caa65e, which is present in this clone. Staged generation now skips the stamp helpers. A new tier-1 test fails with the fix reverted and passes on the head. tier4-e2e job 3478 ran on this head, upgraded and applied the branch on a rig, and succeeded with 4/4 rows. All 10 GitHub checks are green. Both round-1 return findings are closed.

  • git cat-file -e 0caa65e: present. git log --no-merges 0caa65e..32e85ec: one commit, 32e85ec 'Skip the apply stamp for staged generation', touching Makefile, docs/operations.md, rigforge.sh, tests/test-config-apply-state.sh and util/config-apply-state.sh (+24/-4).

  • Round 1 [return] stage path: FIXED. util/config-apply-state.sh:14 [ "${1:-}" != stage ] || return 0 in _config_apply_begin and :24 the same guard on $2 in _config_apply_stamp. rigforge.sh:1030 and :1193 pass the mode. The callers of generate_xmrig_config (rigforge.sh:1005 stage, :2000, :2154, :4151 non-stage) show the non-stage run after the swap still clears and writes the stamp.

  • bash tests/test-config-apply-state.sh on head: exit 0, 'config apply state: PASS'.

  • Scratch worktree at 32e85ec with the rigforge.sh and util hunks from 0caa65e..32e85ec reverse-applied (git apply -R, exit 0): the same test exits 1 with 'FAIL: stage generation leaves the live apply stamp untouched'. The test is registered in tests/run.sh:45.

  • Round 1 [return] missing tier4-e2e: ANSWERED. bench-ci evidence 3478@32e85ec46f19c9a3fc99117b954a8ebd6053e76c printed a block identical to the PR body's: rigforge tier4-e2e, phases connect and dev-fee, completed, conclusion success, exit 0, 4 pass, 0 fail, 0 skip, baseline verified, rig restored. rigforge tier4-e2e is selected by phase and has no mode (per bench-ci tiers).

  • bench-ci log 3478: the run checks out 32e85ec in the rig's install tree. The dev-fee phase runs a candidate upgrade, which builds, runs staged 'Generating hardware-optimized XMRig configuration' and then the live generation after 'Archiving existing worker installation'. It then runs an apply ('Applied config.'). No mktemp error appears after the staged generation line. The phase passed 1 assertion and connect passed 3. The job concluded success.

  • gh pr checks 609: 10/10 pass at head. These are both lint jobs, markdownlint, the test suite, end-to-end (Docker), coverage (kcov + diff-cover), both stock home setups, zizmor and gitleaks. None were skipped.

  • FILE_BUDGET_REQUIRE_BASE=1 bash scripts/lint-file-budget.sh: exit 0, 'file budget OK'. make lint-topology: exit 0.

  • detect-topology.py over the PR Warn when source configuration changes after generation #609 body and comments and the issue doctor cannot tell that the miner runs a stale generated config after a hand edit + restart — record an apply stamp and warn on it #445 comments: exit 0, 'clean'. The PR has no review comments.

  • Round 1 notes: Makefile:35 coverage description now names util/config-apply-state.sh (fixed). The docs/operations.md doctor/status sentence is now its own paragraph (fixed). The ci.yml:189 wording, the unguarded source and the doctor-test extraction remain notes; the worker answered the first two in the body. The base develop-v1.19.0 is unchanged and remains a note.

  • Comment discipline: the new hunks add no source comments. The docs/operations.md line added in this commit describes the stage behaviour accurately. The new diff is two guard lines and two argument passes, with nothing to cut.

  • .github/workflows/ci.yml:189: The coverage description still reads 'rigforge.sh + util/proposed-grub.sh' although kcov now also includes util/config-apply-state.sh (tests/coverage.sh:55). The wording is stale; it does not change behaviour.

  • tests/test-config-apply-state.sh (doctor row): The doctor row is still asserted by extracting the doctor prefix with declare -f | awk instead of running doctor. This round-1 note was not addressed and does not block the merge.

Reviewed head: 32e85ec46f19c9a3fc99117b954a8ebd6053e76c.

@pithead-developer pithead-developer Bot added the stage/awaiting-merge the reviewer's PASS is on the head; merging when checks and the base allow label Nov 7, 2026
@pithead-developer pithead-developer Bot added stage/interrupted stage/in-progress and removed stage/awaiting-merge the reviewer's PASS is on the head; merging when checks and the base allow stage/interrupted labels Nov 7, 2026
Co-Authored-By: Claude Code <noreply@anthropic.com>
@pithead-developer pithead-developer Bot added stage/bench-testing its issue waits on a queued or running tier-4 bench job stage/in-progress and removed stage/in-progress stage/bench-testing its issue waits on a queued or running tier-4 bench job labels Nov 7, 2026
@pithead-reviewer

Copy link
Copy Markdown
Contributor

PASS

What I investigated

This is a round-2 re-review of 0caa65e..41223ca. The worker's only own commit is 32e85ec; the rest arrived with the develop-v1.19.0 merge, and 0caa65e is present in this clone. Both round-1 return findings are fixed. Staged generation no longer touches the live apply stamp, and a tier-1 test that goes red with the fix reverted backs this up. The quoted tier4-e2e job 3563, rerun by me with bench-ci evidence, ran on the current head, built and deployed the branch on the rig, and passed. The merge's conflict resolution in doctor, svc_status and generate_xmrig_config keeps both sides. All ten GitHub checks are green.

  • git cat-file -e 0caa65e: present. git log --no-merges 0caa65e..41223ca: the worker's only commit is 32e85ec 'Skip the apply stamp for staged generation'; a985c6e, cbca1a0, 65f4587 and 39526ae came in with the base merge.

  • Round-1 finding 1 (stage path touches the live stamp, mktemp error): FIXED. util/config-apply-state.sh:14 and :24 return early for stage, and rigforge.sh passes ${1:-} to both helpers (lines 1030 and 1193 at 32e85ec). New cases are at tests/test-config-apply-state.sh:217-230.

  • Green: bash tests/test-config-apply-state.sh on the extracted 41223ca tree exited 0 and printed 'config apply state: PASS'.

  • Red: with the rigforge.sh and util hunks of 32e85ec reverted (patch -R), the test exited 1 with 'FAIL: fresh-install stage generation is silent on stderr'.

  • Red, one guard only: with only the _config_apply_begin stage guard removed, the test exited 1 with 'FAIL: stage generation leaves the live apply stamp untouched'. With only the _config_apply_stamp guard removed, it exited 0 and printed PASS (see note).

  • Round-1 finding 2 (no tier4-e2e on head): FIXED. bench-ci evidence 3563@41223ca896342d7a4ea3ad820e0ebbe270da7f7c exited 0 and printed a block identical to the PR body: tier4-e2e, phases connect and dev-fee, completed, conclusion success, exit 0, steps all success, rows 4 pass 0 fail 0 skip.

  • The bench-ci log 3563 run step reads 'fetch and check out 41223ca… in the rig's install tree'. The dev-fee phase then compiled XMRig from the branch, logged 'Generating hardware-optimized XMRig configuration' for the staged build and again after archiving, installed the unit, and later 'Applied config'. No mktemp error appears anywhere in the log. Phase connect passed 3 assertions and dev-fee passed 1. The rig was restored and the conclusion is success.

  • gh pr checks 609: all 10 pass at the head, none skipped. They are Lint (shellcheck + shfmt), yamllint, markdownlint, zizmor, gitleaks, Test suite, Stock home setup (historical and current), End-to-end (Docker), and Coverage (kcov + diff-cover).

  • git show --cc 41223ca: the merge keeps _shared_host_config before _config_install followed by the stamp call, _shared_host_report plus _config_apply_check in svc_status, and the hoisted local issues=0 with the shared-host block plus _config_apply_check in doctor. The two-line parse_config comment the merge drops was already absent on the PR side (0 hits in 32e85ec and 0caa65e), so dropping it is consistent with the PR's own hunk.

  • FILE_BUDGET_REQUIRE_BASE=1 bash scripts/lint-file-budget.sh: exit 0, 'file budget OK'.

  • detect-topology.py over the PR Warn when source configuration changes after generation #609 body and comments and the issue doctor cannot tell that the miner runs a stale generated config after a hand edit + restart — record an apply stamp and warn on it #445 comments: 'clean' on both, exit 0.

  • Round-1 notes: the Makefile coverage description now names util/config-apply-state.sh, and the docs/operations.md doctor/status text is now its own paragraph. The worker explained in the body why the ci.yml comment and the unguarded source stay as they are. The advisory-only warn row is by design and documented. None of these blocked.

  • make lint was not run locally: this Mac has shfmt 3.13.1 and CI pins 3.14.1, and CI's pinned Lint job is green at the head.

  • util/config-apply-state.sh:24: The _config_apply_stamp stage guard has no test of its own: with it removed and the begin guard kept, tests/test-config-apply-state.sh still passes. In the real setup and upgrade flows, staged generation runs before any non-stage generation, so CONFIG_TUNE_HASH is empty and the stamp is skipped anyway. This makes the guard defence in depth. The two guards together are covered red and green.

  • tests/test-config-apply-state.sh:222: The 'leaves the live apply stamp untouched' assertion compares the whole stamp, including the one-second generated_at. With both guards reverted, the old code rewrites an identical stamp within the same second, so this line passes and the run only goes red at the fresh-install line (my red run: 'FAIL: fresh-install stage generation is silent on stderr'). The PR body's red claim names the earlier line; tidy the body to match.

Reviewed head: 41223ca896342d7a4ea3ad820e0ebbe270da7f7c.

@pithead-developer pithead-developer Bot added stage/awaiting-merge the reviewer's PASS is on the head; merging when checks and the base allow and removed stage/in-progress labels Nov 7, 2026
@pithead-reviewer

Copy link
Copy Markdown
Contributor

Owner brief for 41223ca89634

What it does. After each config generation, rigforge now records SHA-256 fingerprints of the source config.json and the tune overlay, plus a UTC timestamp, in a small stamp file beside the generated XMRig config. doctor and status compare the stamp with the current files. A match shows OK. A change shows a warning telling the operator to run sudo ./rigforge.sh apply. A missing or invalid stamp shows an informational note. The generated config and the API revision are unchanged.

Why. Closes #445 (milestone v2.x post-GA, not a must-fix). Before this, if you edited config.json or the tune overlay and restarted the service by hand, the miner kept the old generated config. Nothing on the rig said that apply was needed.

Risk. Low. rigforge.sh now unconditionally sources a new util file at startup, so every command, help included, aborts if util/ is missing. That is fine for a git checkout. The new row only warns and does not change the exit status of doctor or status. Generation now deletes the old stamp first and rewrites it afterwards. Edits that change and then revert during generation go undetected, and the docs say so. Whitespace-only edits and watchdog-only edits also trigger the warning. The stamp is written on the real setup, upgrade and apply paths, but the staged-build path skips it. The make coverage failure in the PR body is a pre-existing donation-test problem with the kcov image and also happens on the original code.

Proof. CI: 11 success. Reviewer: PASS at 41223ca by pithead-reviewer, round 2 (recorded 2026-11-07T19:57:35Z). Bench: job 3563: success on the head.

What to check.

  • util/config-apply-state.sh:12-37: the stamp is cleared before generation and written only if the input fingerprints still match. Both stage-mode guards are here.
  • util/config-apply-state.sh:40-66: the check logic and its OK, warn and info outcomes, including the handling of an unreadable tune overlay or stamp.
  • rigforge.sh:1039 and :1183: where the begin and stamp calls sit in generate_xmrig_config. rigforge.sh:65 is the unguarded source of the util file.
  • tests/test-config-apply-state.sh:~218-232: the stage-mode regressions. The reviewer noted that the 'live stamp untouched' line compares the whole stamp, so only the fresh-install line fails if both guards are reverted.

Verdict. merge

VijitSingh97
VijitSingh97 previously approved these changes Nov 8, 2026
@pithead-developer pithead-developer Bot added stage/in-progress and removed stage/awaiting-merge the reviewer's PASS is on the head; merging when checks and the base allow labels Nov 8, 2026
Co-Authored-By: Claude Code <noreply@anthropic.com>
@pithead-reviewer

Copy link
Copy Markdown
Contributor

PASS

What I investigated

Round-3 re-review of #609 (closes #445) at head 95725e5. 0caa65e is in this clone. The worker's only new commit since then is 32e85ec. Everything else arrived through merges of develop-v1.19.0. Both earlier return findings are fixed. The stage path no longer touches the live apply stamp, and a tier-1 test covers it: red with the fix reverted, green on the head. tier4-e2e job 3589 ran on the head and succeeded, including a staged rebuild. All ten GitHub checks are green.

  • git cat-file -e 0caa65e succeeded. git log --no-merges 0caa65e..95725e5 lists one worker commit, 32e85ec 'Skip the apply stamp for staged generation'. The other five are base squash-merges (Add an opt-in shared-host resource profile #611, Automate docs replay, API allowlist, and uninstall checks #617, Expose appliance mode and baked ref in the sister feed #637, Qualify historical RigForge benchmark claims #639, Support solo daemon pools and report node health #645).

  • Round 1 finding (stage path touches the live stamp): FIXED. util/config-apply-state.sh:14 [ "${1:-}" != stage ] || return 0 in _config_apply_begin, util/config-apply-state.sh:24 [ "${2:-}" != stage ] || return 0 in _config_apply_stamp, and rigforge.sh:1035/1197 pass the mode. setup (rigforge.sh:1997) and upgrade (rigforge.sh:2152) still call generate_xmrig_config without stage after compile_xmrig, so the live build is stamped after activation.

  • Green: bash tests/test-config-apply-state.sh at the head in a scratch worktree printed exit=0 and 'config apply state: PASS'.

  • Red: in a scratch worktree under $TMPDIR at the head I reverted only the four non-test lines of 32e85ec (in rigforge.sh and util/config-apply-state.sh). bash tests/test-config-apply-state.sh then exited 1 with 'FAIL: stage generation leaves the live apply stamp untouched'.

  • The test is registered in the aggregate suite at tests/run.sh:45 (bash "$ROOT/tests/test-config-apply-state.sh" "$SCRIPT"), so tier-1 CI runs it.

  • Round 1 finding (no tier4-e2e on the head): FIXED. bench-ci evidence 3589@95725e5445d77e2726bf78c269f366334f071984 exited 0 and prints the same block the PR quotes: tier4-e2e, phases connect and dev-fee, conclusion success, exit 0, baseline verified, rig restored, all steps success, rows 4 pass / 0 fail / 0 skip.

  • bench-ci log 3589 exited 0 (113 lines). The job header names rigforge@95725e5. The run step checks out 95725e5 in the rig's install tree. Phase connect passed 3 assertions. Phase dev-fee compiled XMRig, then logged 'Generating…' (staged), 'Archiving existing worker installation...', 'Generating…' (live) and passed 1 assertion. grep -ci 'mkstemp|No such file' on the log returned 0.

  • gh pr checks 609 --json: all 10 pass, none skipped. They are Lint (markdownlint), Lint (shellcheck + shfmt), Lint (yamllint), Coverage (kcov + diff-cover), End-to-end (Docker), Secret scan (gitleaks), Test suite, Workflow audit (zizmor), Stock home setup (historical) and Stock home setup (current).

  • FILE_BUDGET_REQUIRE_BASE=1 bash scripts/lint-file-budget.sh printed 'file budget OK' with exit 0.

  • Comment discipline: git diff 4ae7fb8 HEAD | grep -E '^\+\s*#' returns nothing apart from the shellcheck directive and shebang, so no explanatory comments were added. The diff against the merge base is 10 files with 335 insertions, all stamp, check, test or docs.

  • Topology: detect-topology.py reports 'clean' with exit 0 over the PR body, comments and reviews and over issue doctor cannot tell that the miner runs a stale generated config after a hand edit + restart — record an apply stamp and warn on it #445's comments. As a control, it exits 1 on a probe file containing an address and a home path.

  • Earlier notes: Makefile:35 coverage description now names util/config-apply-state.sh (fixed). The docs/operations.md doctor/status sentence is now its own paragraph (fixed). The ci.yml comment, the unguarded source and the advisory-only warn row were answered in the PR body and stay notes, not findings.

  • make lint was not re-run locally: this Mac has shfmt 3.13.1 and CI pins 3.14.1. CI's pinned Lint (shellcheck + shfmt) job is green at the head. No Docker was run locally.

  • PR Warn when source configuration changes after generation #609 body, 'Bench on the previous head (41223ca)' block: The job 3563 block is for an older head and is superseded by job 3589 on the current head. It can be dropped when the body is next tidied.

  • tests/test-config-apply-state.sh (doctor row): Round-1 note still applies and is not blocking: the doctor row is asserted by extracting a function prefix with declare -f | awk rather than by running doctor.

Reviewed head: 95725e5445d77e2726bf78c269f366334f071984.

@pithead-developer pithead-developer Bot added stage/awaiting-merge the reviewer's PASS is on the head; merging when checks and the base allow and removed stage/in-progress labels Nov 8, 2026
@pithead-reviewer

Copy link
Copy Markdown
Contributor

Owner brief for 95725e5445d7

What it does. doctor and status now show one extra line saying whether the source config.json and tune overlay still match what the last config generation used. If either changed, they warn and tell the operator to run sudo ./rigforge.sh apply. If no record exists (older installs), the line is informational only. Generation writes a small stamp file with SHA-256 fingerprints of both source files and a timestamp beside the generated config.

Why. Closes #445 (milestone v2.x post-GA). Before this, you could edit the source config, restart the service by hand, and the miner came back on the old generated config. Nothing on the rig said apply was needed.

Risk. Low. The generated config and the API revision are unchanged. The warning is advisory and does not change the exit status of doctor or status. Two things to know. rigforge.sh now unconditionally sources a new util file at startup, so every verb, help included, fails if util/ is missing. That is fine for a git checkout. The check compares source files, not what the process loaded, so whitespace edits warn too. Editing the sources while generation runs is not detected if the edit reverts. The make coverage run in the PR failed on an existing donation test, because the coverage image's lscpu output lacks a model row. The PR says the same failure happens on the base commit.

Proof. CI: 11 success. Reviewer: PASS at 95725e5 by pithead-reviewer, round 2 (recorded 2026-11-08T02:46:15Z). Bench: job 3563: success on 41223ca, not the head; job 3589: not recorded on the head.

What to check.

  • util/config-apply-state.sh:11-37: stamp begin/write logic. It deletes the old stamp first and writes a new one only if the input fingerprints still agree. Staged builds skip both steps, so setup or upgrade never touches the live stamp.
  • rigforge.sh:66 (the unguarded source of the new util file) and rigforge.sh:1036 and :1180 (the two generation hooks).
  • util/config-apply-state.sh:39-66: the check function and its warn/info/ok wording, including how it handles bad or missing stamps.
  • The coverage failure: confirm you accept it as a pre-existing donation-test and lscpu problem in the pinned kcov image, since the coverage-floor check never ran.

Verdict. merge: the reviewer passed this at the current head. The stage-path bug found in round 1 is fixed and has a test. The two test notes are non-blocking

@pithead-reviewer
pithead-reviewer Bot merged commit e4f3ea9 into develop-v1.19.0 Nov 8, 2026
11 checks passed
@pithead-reviewer
pithead-reviewer Bot deleted the astra/445-rigforge branch November 8, 2026 03:00
pithead-developer Bot pushed a commit that referenced this pull request Nov 8, 2026
The base grew solo daemon pools (#645), the source-drift apply-state
check (#609) and JSON doctor reports (#606) after the first sync, which
the bench caught as a conflict with the tested head. Resolutions: keep
the xmrig overlay alongside the new daemon-pools source and pool keys;
keep the overlay doctor line next to the new apply-state check; keep
the broadened util/ coverage denominator over the individually named
helpers. tests/test-config-apply-state.sh gains a validator stub in its
fixture because apply now requires a validation executable; its
assertions are unchanged.

Co-Authored-By: Claude Code <noreply@anthropic.com>
pithead-developer Bot pushed a commit that referenced this pull request Nov 8, 2026
The #609 generated-configuration report was the only doctor output
emitted without a check ID, so the #645 daemon-pool doctor-JSON
assertions (all IDs nonempty) turned red once both landed on the base.
Label it apply-state; human messages and exit codes unchanged.

Co-Authored-By: Claude Code <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

stage/awaiting-merge the reviewer's PASS is on the head; merging when checks and the base allow

Projects

None yet

Development

Successfully merging this pull request may close these issues.

doctor cannot tell that the miner runs a stale generated config after a hand edit + restart — record an apply stamp and warn on it

1 participant