Skip to content

Commit 15fc0ea

Browse files
authored
fix: two affirmatives nobody had earned (#227)
45k — the Active Services card showed "0" when the count could not be taken. _get_all_worker_containers opens SQLite, so a locked or busy database, or a JSON-decode failure on a worker row, lands in the except while containers are in fact running. The fallback was 0, which reads as "nothing is running", and the only other signal was logger.debug — DEBUG is off in production, so both places that could have said something said nothing. It is None now, rendered as an em dash, and the failure is logged at WARNING. tb5 — the bell rendered an empty alert list as "All collectors healthy". On a fresh install, or after a restart before the first hourly collection, nothing has been checked. The bell's own FAILURE path is written correctly ("Alerts unavailable" rather than healthy), which makes the never-ran case the outlier rather than a matter of style. /api/collector-alerts returns {alerts, collected} rather than a bare list, so "no alerts" and "nothing has run" can be told apart at all. The UI is the only consumer and ships with this. `collected` is set even on a FAILED run: the bell's question is "has anything looked", and a run that tried and failed has looked — its failure is in the list the bell is about to show. It is also restored at startup from stored alerts or any earnings row, so a restart does not reset the claim to "never ran" while data exists, and a lookup that itself fails leaves it False rather than asserting a run happened. Negative control: the count falling back to 0 fails 1 test; the failure logged at DEBUG fails 1; the endpoint hardcoding collected fails 1; the bell claiming health unconditionally fails 1; dropping the restart restore fails 2. Two existing tests asserted the old contracts (active_services == 0, and a bare list from the endpoint). Both now assert the new ones with the reason recorded. Closes CashPilot-45k Closes CashPilot-tb5
1 parent 3fd0dd6 commit 15fc0ea

5 files changed

Lines changed: 229 additions & 12 deletions

File tree

app/main.py

Lines changed: 46 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,12 @@
6363

6464
# In-memory store for the latest collector alerts (errors from last run)
6565
_collector_alerts: list[dict[str, str]] = []
66+
# Whether a collection has ever COMPLETED here. An empty alert list means
67+
# "nothing is wrong" only once something has looked; before that it means
68+
# "nothing has been checked", and the bell used to render both as "All
69+
# collectors healthy" (CashPilot-tb5). Restored on startup from durable state so
70+
# a restart does not reset the claim to "never ran" while data exists.
71+
_collection_has_run: bool = False
6672
_collection_lock = asyncio.Lock()
6773
_collection_semaphore = asyncio.Semaphore(8)
6874

@@ -508,6 +514,16 @@ async def _warm_collector_alerts() -> None:
508514
seen.add(key)
509515
restored.append({"kind": alert["kind"], "platform": alert["subject"], "error": alert["message"]})
510516
_collector_alerts = restored
517+
# A restart must not make the bell claim nothing has ever been checked. Any
518+
# stored alert, or any earnings row, is proof that a collection ran.
519+
global _collection_has_run
520+
if restored:
521+
_collection_has_run = True
522+
else:
523+
try:
524+
_collection_has_run = bool(await database.get_earnings_summary())
525+
except Exception as exc: # noqa: BLE001 - a warm-up must not block startup
526+
logger.warning("Could not tell whether a collection has run before: %s", exc)
511527

512528

513529
async def _run_collection() -> None:
@@ -633,6 +649,11 @@ async def _run_collection() -> None:
633649
]
634650
finally:
635651
metrics.record_collection_end(start_time, success, platforms_ok)
652+
# Set even on a failed run: the bell's question is "has anything
653+
# looked", and a run that tried and failed HAS looked — its failure
654+
# is in the alert list the bell is about to show.
655+
global _collection_has_run
656+
_collection_has_run = True
636657

637658

638659
# ---------------------------------------------------------------------------
@@ -1941,13 +1962,20 @@ async def api_earnings_summary(request: Request) -> dict[str, Any]:
19411962
total_adjusted += adjusted
19421963
total_bonus_usd += bonus
19431964

1944-
# Count active (running) services from worker data
1945-
active = 0
1965+
# Count active (running) services from worker data.
1966+
#
1967+
# None, not 0, when the count could not be taken. _get_all_worker_containers
1968+
# opens SQLite, so a locked or busy database — or a JSON-decode failure on a
1969+
# worker row — lands here while containers are in fact running, and "0"
1970+
# reads as "nothing is running". Logged at WARNING rather than DEBUG: DEBUG
1971+
# is off in production, so the only two places that could have said anything
1972+
# both said nothing (CashPilot-45k).
1973+
active: int | None = None
19461974
try:
19471975
worker_containers = await _get_all_worker_containers()
19481976
active = sum(1 for s in worker_containers if s.get("status") == "running")
19491977
except Exception as exc:
1950-
logger.debug("active-service count failed: %s", exc)
1978+
logger.warning("Could not count active services, reporting it as unknown: %s", exc)
19511979
summary["active_services"] = active
19521980
# A total that silently omits holdings is indistinguishable from a correct
19531981
# one. The count was already being computed in database.py and only logged;
@@ -2870,8 +2898,20 @@ async def api_disclosure_coverage(request: Request) -> dict[str, Any]:
28702898

28712899

28722900
@app.get("/api/collector-alerts")
2873-
async def api_collector_alerts(request: Request) -> list[dict[str, str]]:
2874-
"""Return collector errors from the last collection run (sanitized)."""
2901+
async def api_collector_alerts(request: Request) -> dict[str, Any]:
2902+
"""Collector errors from the last run, and whether a run has happened.
2903+
2904+
The bell rendered an empty list as "All collectors healthy". On a fresh
2905+
install — or after a restart, before the first hourly collection — nothing
2906+
has been checked, so that affirmative is unearned: it is the same
2907+
absent-equals-true shape this codebase rejects everywhere else. The bell's
2908+
own FAILURE path is written correctly ("Alerts unavailable" rather than
2909+
healthy), which makes the never-ran case the outlier (CashPilot-tb5).
2910+
2911+
Returning an object rather than a bare list so "no alerts" and "nothing has
2912+
run" can be told apart at all. The UI is the only consumer and ships with
2913+
this.
2914+
"""
28752915
_require_auth_api(request)
28762916
sanitized: list[dict[str, str]] = []
28772917
for alert in _collector_alerts:
@@ -2882,7 +2922,7 @@ async def api_collector_alerts(request: Request) -> list[dict[str, str]]:
28822922
# `kind` is additive and defaults to "collector", so a frontend that
28832923
# does not read it behaves exactly as before.
28842924
sanitized.append({"kind": alert.get("kind", "collector"), "platform": alert["platform"], "error": clean})
2885-
return sanitized
2925+
return {"alerts": sanitized, "collected": _collection_has_run}
28862926

28872927

28882928
@app.get("/api/exchange-rates")

app/static/js/app.js

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -488,7 +488,10 @@ const CP = (() => {
488488
setTextContent('total-earnings', money(displayTotal));
489489
setTextContent('today-earnings', money(data.today || 0));
490490
setTextContent('month-earnings', money(data.month || 0));
491-
setTextContent('active-services', data.active_services || 0);
491+
// `|| 0` would render "could not be counted" as "nothing is running".
492+
// The endpoint sends null when the count could not be taken
493+
// (CashPilot-45k).
494+
setTextContent('active-services', data.active_services == null ? '\u2014' : data.active_services);
492495

493496
const nothingYet = document.getElementById('no-readings-note');
494497
if (nothingYet) nothingYet.style.display = data.has_readings === false ? '' : 'none';
@@ -2934,14 +2937,23 @@ const CP = (() => {
29342937
if (!container || !badge || !list) return;
29352938

29362939
try {
2937-
const alerts = await api('/api/collector-alerts');
2940+
const payload = await api('/api/collector-alerts');
2941+
const alerts = payload.alerts || [];
29382942
// Clear any muted "alerts unavailable" styling left over from a prior
29392943
// failed poll now that the fetch succeeded.
29402944
badge.style.background = '';
29412945
badge.title = '';
2942-
if (!alerts || alerts.length === 0) {
2946+
if (alerts.length === 0) {
29432947
badge.style.display = 'none';
2944-
list.innerHTML = '<div class="notify-empty">All collectors healthy</div>';
2948+
// "Healthy" is a claim about something that was CHECKED. On a fresh
2949+
// install, or after a restart before the first collection, nothing has
2950+
// been — and saying so is the same absent-equals-true this codebase
2951+
// rejects everywhere else. The bell's failure path already gets this
2952+
// right ("Alerts unavailable"), which made the never-ran case the
2953+
// outlier (CashPilot-tb5).
2954+
list.innerHTML = payload.collected
2955+
? '<div class="notify-empty">All collectors healthy</div>'
2956+
: '<div class="notify-empty">No collection has run yet — nothing has been checked.</div>';
29452957
return;
29462958
}
29472959

tests/test_audit_guards.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -451,7 +451,9 @@ def test_the_endpoint_tags_every_alert_so_the_bell_can_tell_them_apart(self):
451451
patch.object(main, "_require_auth_api", lambda r: None),
452452
):
453453
out = asyncio.run(main.api_collector_alerts(MagicMock()))
454-
assert out[0]["kind"] == "collector", "an untagged alert must default to collector, not vanish"
454+
# The endpoint returns {alerts, collected} so "no alerts" and "nothing
455+
# has run" can be told apart at all (CashPilot-tb5).
456+
assert out["alerts"][0]["kind"] == "collector", "an untagged alert must default to collector, not vanish"
455457

456458

457459
class TestTheCredentialCooldownCannotBeRaced:

tests/test_beads_batch_41.py

Lines changed: 160 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,160 @@
1+
"""CashPilot-45k and -tb5: two affirmatives nobody had earned.
2+
3+
**45k** — the Active Services card showed "0" when the count could not be taken.
4+
``_get_all_worker_containers`` opens SQLite, so a locked or busy database, or a
5+
JSON-decode failure on a worker row, lands in the ``except`` while containers
6+
are in fact running. The fallback was ``0``, which reads as "nothing is
7+
running", and the only other signal was a ``logger.debug`` — and DEBUG is off in
8+
production, so both places that could have said something said nothing.
9+
10+
**tb5** — the bell rendered an empty alert list as "All collectors healthy". On
11+
a fresh install, or after a restart before the first hourly collection, nothing
12+
has been checked. The bell's own FAILURE path is written correctly ("Alerts
13+
unavailable" rather than healthy), which makes the never-ran case the outlier
14+
rather than a matter of style.
15+
16+
Both are the same shape: absent presented as an affirmative.
17+
"""
18+
19+
from __future__ import annotations
20+
21+
import re
22+
from pathlib import Path
23+
from unittest.mock import AsyncMock, MagicMock, patch
24+
25+
import pytest
26+
27+
ROOT = Path(__file__).resolve().parents[1]
28+
APP_JS = ROOT / "app" / "static" / "js" / "app.js"
29+
30+
31+
def js() -> str:
32+
text = APP_JS.read_text(encoding="utf-8")
33+
text = re.sub(r"/\*.*?\*/", "", text, flags=re.S)
34+
return "\n".join(re.sub(r"(^|\s)//.*$", "", line) for line in text.splitlines())
35+
36+
37+
async def _summary(*, worker_read_fails):
38+
from app import main
39+
40+
containers = (
41+
AsyncMock(side_effect=Exception("database is locked")) if worker_read_fails else AsyncMock(return_value=[])
42+
)
43+
with (
44+
patch.object(main, "_require_auth_api", lambda r: None),
45+
patch.object(main.database, "get_earnings_dashboard_summary", AsyncMock(return_value={"total": 0})),
46+
patch.object(main.database, "get_config", AsyncMock(return_value={})),
47+
patch.object(main.database, "get_earnings_summary", AsyncMock(return_value=[])),
48+
patch.object(main, "_get_all_worker_containers", containers),
49+
):
50+
return await main.api_earnings_summary(MagicMock())
51+
52+
53+
class TestAnUncountableFleetIsUnknown:
54+
@pytest.mark.asyncio
55+
async def test_a_failed_read_reports_none(self):
56+
assert (await _summary(worker_read_fails=True))["active_services"] is None
57+
58+
@pytest.mark.asyncio
59+
async def test_a_successful_read_of_nothing_still_reports_zero(self):
60+
"""The control. A fleet with no running containers really is zero."""
61+
assert (await _summary(worker_read_fails=False))["active_services"] == 0
62+
63+
@pytest.mark.asyncio
64+
async def test_the_failure_is_logged_where_it_can_be_seen(self, caplog):
65+
"""DEBUG is off in production, so a debug log is the same as silence."""
66+
import logging
67+
68+
with caplog.at_level(logging.WARNING, logger="app.main"):
69+
caplog.clear()
70+
await _summary(worker_read_fails=True)
71+
assert any("Could not count active services" in r.getMessage() for r in caplog.records)
72+
73+
def test_the_card_renders_a_dash(self):
74+
source = js()
75+
assert "data.active_services || 0" not in source, "unknown still renders as 0"
76+
assert "data.active_services == null" in source
77+
78+
79+
class TestTheBellDoesNotClaimUncheckedHealth:
80+
async def _alerts(self, *, alerts, has_run):
81+
from app import main
82+
83+
with (
84+
patch.object(main, "_require_auth_api", lambda r: None),
85+
patch.object(main, "_collector_alerts", alerts),
86+
patch.object(main, "_collection_has_run", has_run),
87+
):
88+
return await main.api_collector_alerts(MagicMock())
89+
90+
@pytest.mark.asyncio
91+
async def test_a_fresh_install_reports_that_nothing_has_run(self):
92+
out = await self._alerts(alerts=[], has_run=False)
93+
assert out["collected"] is False
94+
assert out["alerts"] == []
95+
96+
@pytest.mark.asyncio
97+
async def test_after_a_run_with_no_problems_it_says_so(self):
98+
"""The control: a genuine all-clear must remain expressible."""
99+
out = await self._alerts(alerts=[], has_run=True)
100+
assert out["collected"] is True
101+
102+
@pytest.mark.asyncio
103+
async def test_the_alerts_still_come_through_tagged(self):
104+
out = await self._alerts(alerts=[{"platform": "grass", "error": "boom"}], has_run=True)
105+
assert out["alerts"][0]["kind"] == "collector"
106+
107+
def test_the_ui_distinguishes_the_two(self):
108+
source = js()
109+
assert "payload.collected" in source
110+
assert "No collection has run yet" in source
111+
112+
def test_the_all_clear_is_still_possible(self):
113+
"""The control: the affirmative must survive for the case that earns it."""
114+
assert "All collectors healthy" in js()
115+
116+
def test_the_ui_reads_the_new_shape(self):
117+
source = js()
118+
assert "payload.alerts" in source
119+
120+
121+
class TestTheFlagSurvivesARestart:
122+
"""A restart must not make the bell claim nothing has ever been checked."""
123+
124+
async def _warm(self, *, stored_alerts, earnings):
125+
from app import main
126+
127+
with (
128+
patch.object(main, "_collection_has_run", False),
129+
patch.object(main.database, "list_alerts", AsyncMock(return_value=stored_alerts)),
130+
patch.object(main.database, "get_earnings_summary", AsyncMock(return_value=earnings)),
131+
):
132+
await main._warm_collector_alerts()
133+
return main._collection_has_run
134+
135+
@pytest.mark.asyncio
136+
async def test_stored_alerts_prove_a_run_happened(self):
137+
stored = [{"kind": "collector", "subject": "grass", "message": "boom"}]
138+
assert await self._warm(stored_alerts=stored, earnings=[]) is True
139+
140+
@pytest.mark.asyncio
141+
async def test_earnings_rows_prove_it_too(self):
142+
rows = [{"platform": "honeygain", "balance": 1.0, "currency": "USD"}]
143+
assert await self._warm(stored_alerts=[], earnings=rows) is True
144+
145+
@pytest.mark.asyncio
146+
async def test_a_genuinely_fresh_install_stays_false(self):
147+
"""The control: without it this passes by always claiming a run."""
148+
assert await self._warm(stored_alerts=[], earnings=[]) is False
149+
150+
@pytest.mark.asyncio
151+
async def test_a_failed_lookup_does_not_claim_a_run(self):
152+
from app import main
153+
154+
with (
155+
patch.object(main, "_collection_has_run", False),
156+
patch.object(main.database, "list_alerts", AsyncMock(return_value=[])),
157+
patch.object(main.database, "get_earnings_summary", AsyncMock(side_effect=Exception("locked"))),
158+
):
159+
await main._warm_collector_alerts()
160+
assert main._collection_has_run is False

tests/test_coverage_gaps.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -820,7 +820,10 @@ def test_earnings_summary_worker_exception(self, client):
820820
):
821821
resp = client.get("/api/earnings/summary")
822822
assert resp.status_code == 200
823-
assert resp.json()["active_services"] == 0
823+
# None, not 0. The count could not be TAKEN — reporting zero here
824+
# says "nothing is running" about a fleet nobody could read
825+
# (CashPilot-45k).
826+
assert resp.json()["active_services"] is None
824827

825828

826829
class TestMainServicesDeployedMultiStatus:

0 commit comments

Comments
 (0)