fix(miner): exit 2, not 1, on a secret-mount failure - #6248
Conversation
bin/loopover-miner.js's loadMinerFileSecrets catch exited 1, but docs/unattended-scheduling.md's contract only defines 0 (success) and 2 (failure -- "Alert on this"). An operator wiring alerting strictly to exit code 2, as that doc instructs, silently missed a broken secret mount (a bad GITHUB_TOKEN_FILE or similar). Checked every other exit in this file: the secret-mount catch was the only hardcoded exit 1: the rest pass a subcommand's own resolved code or 0. lib/env-file-indirection.js is untouched -- the throw is correct, only the CLI-boundary translation was wrong. The existing end-to-end test pinned status 1, so it is updated to assert the documented 2 (it spawns the real bin against a missing GITHUB_TOKEN_FILE). Closes JSONbored#6162
|
🚨 Contributor flagged. Click here for more info: Superagent Dashboard |
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6248 +/- ##
=======================================
Coverage 95.35% 95.35%
=======================================
Files 598 598
Lines 47102 47102
Branches 14988 14988
=======================================
Hits 44913 44913
Misses 1464 1464
Partials 725 725
Flags with carried forward coverage won't be shown. Click here to find out more. |
|
Caution 🛑 LoopOver review result - reject/close recommendedReview updated: 2026-07-15 23:55:34 UTC
Review summary Nits — 5 non-blocking
Why this is blocked
📋 Copy for AI agents — paste into your coding agentFlagged checks (non-blocking)
Decision drivers
Context & advisory signals — never blocks the verdict
Linked issue satisfactionAddressed Review context
Contributor next steps
Signal definitions
🧪 Chat with LoopOverAsk LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.
Full command reference: https://gittensory.aethereal.dev/docs/loopover-commands 🧪 Experimental — new and may change. 🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.
|
|
LoopOver is closing this pull request on the maintainer's behalf (Linked issue overlaps another open PR; duplicate of another open PR). This is an automated maintenance action — to pursue this change, please open a new pull request with the issues resolved. Closed PRs may be analyzed later to improve review accuracy, but they are not automatically reopened or re-reviewed. |
Summary
Closes #6162
bin/loopover-miner.js'sloadMinerFileSecrets()catch calledprocess.exit(1), butpackages/loopover-miner/docs/unattended-scheduling.md:15-18's exit-code contract only defines0(success) and2(failure — "Alert on this"). An operator who wires alerting strictly to exit code2, exactly as that doc instructs, silently missed a real, reachable failure: a brokenGITHUB_TOKEN_FILEsecret mount.2, so the documented alerting catches it.exit(1). Every otherprocess.exitpasses a subcommand's own resolved code (or0), so there is no second miscoded startup path to fix.lib/env-file-indirection.jsis deliberately untouched — its throw is correct; only the CLI-boundary translation was wrong.Scope
type(scope): short summaryConventional Commit format, for examplefix(api): restore profile access checks.CONTRIBUTING.mdand does not reintroduce GitHub Pages, VitePress,site/, orCNAME.Closes #123) — a linked open issue is required for every contributor PR.Validation
git diff --checknpm run actionlintnpm run typechecknpm run test:coveragelocally;codecov/patchrequires ≥99% coverage of the lines AND branches you changed (aim for 100% on your diff so CI variance does not fail near the threshold). Global coverage is a non-blocking trend with a loose 90% backstop, not the gate.npm run test:workersnpm run build:mcpnpm run test:mcp-packnpm run ui:openapi:checknpm run ui:lintnpm run ui:typechecknpm run ui:buildnpm audit --audit-level=moderateIf any required check was skipped, explain why:
GITHUB_TOKEN_FILE. That test pinned the oldstatus === 1, so it is updated to assert the documented2(plus an explicitnot.toBe(1)regression guard). This is the test the issue's deliverable asks for; a second one asserting the same spawn would be redundant.miner-env-file-indirection(13 tests, includes the real-bin spawn), plusminer-package-skeletonandcheck-miner-package(18 tests) as the other bin/package consumers.Safety
UI Evidencesection below with JPG/JPEG or PNG screenshots arranged as organized, captioned, clickable thumbnails. SVG screenshots are not used as review evidence. Review-only screenshots or recordings are not committed to the repository.The failure path is a credential-mount error: the test asserts the error names the offending var and its path, and no secret value is ever printed. This only changes the process exit code, not what is emitted.
Notes
Closes #6162