fix(worker): removing a worker no longer locks the host out permanently - #197
Conversation
Removing a worker in the fleet dashboard deletes its row and its per-worker key on the UI side. The worker never learned that. `_active_key()` returns `_worker_key or API_KEY`, so once /data/.worker_key exists the shared key is never sent again — the host 401s forever while its service containers keep running and keep earning, and nothing on either side says why. Both places that described this promised a recovery that no code performed: docs/upgrade-v1.md said removing the worker "clears its enrollment so the shared key is accepted again", and the confirm dialog said only "This will unregister it from the fleet." The real fix was SSHing in to delete a file documented nowhere. After ten consecutive rejections of our own key the worker now discards it and re-enrols. Bounded on purpose: discarding on the first failure would re-enrol on any transient blip and widen the window in which the shared key is accepted, which is what per-worker keys exist to close. The threshold sits above the existing alarm, so the operator is told before anything is deleted. A key that cannot be removed from disk is kept rather than zeroed in memory — believing we re-enrolled while the stale key survives a restart is worse than not trying. Negative control: removing the five-line discard branch fails test_sustained_rejection_discards_the_stale_key and test_it_says_why_in_the_log and nothing else; reverting the two doc changes fails their two tests. Closes CashPilot-u10
|
Warning Review limit reached
Next review available in: 22 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #197 +/- ##
=======================================
Coverage 95.09% 95.10%
=======================================
Files 46 46
Lines 5893 5905 +12
=======================================
+ Hits 5604 5616 +12
Misses 289 289
🚀 New features to boost your workflow:
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai review |
✅ Action performedReview finished.
|
1 participant
The defect
Removing a worker in the fleet dashboard deletes its row and its per-worker key on the UI side. The worker never learned that.
app/worker_api.py_active_key()returns_worker_key or API_KEY— once/data/.worker_keyexists, the shared key is never sent again. So the host 401s forever, while its service containers keep running and keep earning. Nothing on either side explains it: the UI shows one fewer worker, the worker logs a bare 401.Both places that described this promised a recovery no code performed:
docs/upgrade-v1.md:51— removing the worker "(clears its enrollment) so the shared key is accepted again". Nothing cleared anything.app/templates/fleet.html:346—Remove worker "${name}"? This will unregister it from the fleet.The only real recovery was SSHing in to delete a file documented nowhere.
The fix
After ten consecutive rejections of our own key, the worker discards it and re-enrols with the shared key.
Bounded deliberately. Discarding on the first failure would re-enrol on any transient blip and widen the window in which the shared key is accepted — the exact weakening per-worker keys exist to close. The threshold sits above the existing
_AUTH_FAILURE_ALARM_AFTER = 3, so the operator is told before anything is deleted. A key that cannot be removed from disk is kept, not zeroed in memory: believing we re-enrolled while the stale key survives a restart brings the same lockout back with no failures leading up to it.The docs and the confirm dialog now describe what actually happens, including the manual shortcut.
Evidence
ruff check .+ruff format --check .clean;node --checkon all JStest_sustained_rejection_discards_the_stale_keyandtest_it_says_why_in_the_log, and nothing else. Reverting the two doc changes fails their two tests.401, 500alternating) discards nothing; a success before the threshold saves the key.Closes CashPilot-u10