fix(doctor): name MSR unverifiable causes and cut the log scan (#367) - #387
Merged
Conversation
doctor's MSR block goes silent whenever the guard in front of it fails — no config.json, or a resolved worker root with no xmrig.log yet — and a silently skipped block reads identically to a failed check. That flake broke both #66 MSR assertions in e2e-real.sh verify on a healthy miner-3: re-running the same commit immediately passed clean. - doctor now names which guard input failed: "MSR unverifiable — no config.json, so the worker root couldn't be resolved" when CONFIG_JSON is missing, vs. "MSR unverifiable — no xmrig.log at <path>" once the root resolves but nothing is logged there yet (fresh install, or a copytruncate rotation window on an otherwise healthy rig). Both are advisory (_ck_info), never a counted issue — absence is not failure. - e2e-real.sh verify resolves the same worker root + log path doctor's guard needs, before the #66 assertions run, so a skipped block now fails there as "could not resolve xmrig.log" (with the path) instead of being misread as "MSR not applied". - _msr_log_status awk-scanned the whole log on every doctor call (122MB seen on miner-3). The confirmation line is written at miner START, so on a long-lived worker it sits near the file's beginning — a naive tail would miss it. grep (C-speed) finds every match and tail -1 keeps the same last-match semantics at a fraction of the cost. Tests: three new doctor cases (unresolved config, resolved root with a missing log, line present) plus the surrounding #66/#140 MSR sections — 1704 passed, 0 failed. e2e-real.sh isn't runnable outside a real rig; the new assertion mirrors the file's existing resolver pattern and is shellcheck/shfmt clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #367.
Three parts, following the issue's own suggested direction:
_ck_info, not counted issues): no config.json so the worker root couldn't resolve, vs a resolved root with no xmrig.log at the named path. Absence and failure are now different signals with different wording.grep | tail -1 | awkreplaces the whole-file awk scan (the 122MB case from the issue), keeping last-match semantics; the comment warns off the naive tail-first "optimization" (the MSR line is written at miner start, not near EOF). The no-line-found wording stays honest about copytruncate rotation: not-found is not proof of not-applied.What was run (implementation + independent adversarial re-verification in a fresh context):
bash tests/run.shforeground: 1704 passed, 0 failed, including the three new e2e-real verify: doctor's MSR block intermittently doesn't render, failing both #66 assertions on a healthy rig #367 tests (unresolved config; resolved root with missing log; unverifiable wording absent once the log has a line).🤖 Generated with Claude Code