Skip to content

Commit 55196e8

Browse files
authored
feat(fleet): accept a paired client's own earnings history (#256)
* feat(fleet): accept a paired client's own earnings history A client that has been reading a provider account on its own -- CashPilot Desktop before it was paired -- had no way to hand that history to the server, so the fleet view began on the day of pairing and every earlier day was simply lost from the total. POST /api/workers/earnings-import takes those readings and stores them under the importing client's own source, which the source-aware schema added in the previous change makes possible. Separate series matter here: earnings are clamped deltas between consecutive readings of the same balance, so interleaving two samplers of one account makes every apparent drop clamp to zero and understates the total. Each series is differenced on its own and the results are summed. Two properties carry the security of it: * The source comes from the AUTHENTICATED worker, never the request body, so no client can write into another's history or into the server's own. * Only a fully enrolled worker may import. A caller still presenting the shared enrollment key gets 403 with instructions to heartbeat first -- every worker holds that key, and this writes durable money data. Re-sending a day updates it rather than appending, so a retried or repeated import is safe by construction. Refs: CashPilot-Desktop-xjr * fix(fleet): bound the import body and validate the reading date Two ways the new endpoint could be handed input it would store without complaint. The date was free text. Both delta readers ORDER BY it, so a reading dated 2026-1-2 or 01/02/2026 sorts into the wrong place in its own series and the readings either side then difference against the wrong neighbour. It fails silently, only for the client that sent it, and only in the earned figure -- never in the balance the dashboard shows. It is now required to be YYYY-MM-DD and a real calendar day, so 2026-02-30 is refused rather than stored. The readings list was unbounded. One authenticated client could hand the server an arbitrarily large body to parse and then write row by row; a single compromised worker is enough. Capped at 2000, comfortably above an honest import (the server keeps 400 days and a client chunks at 1000). Validation runs before authentication, so a malformed body cannot be used to probe which client ids exist. Refs: CashPilot-Desktop-xjr * fix(compose): pin the examples to the 1.15 series v1.15.0 released and the example compose files still pinned 1.14, so anyone following the quickstart deployed a series behind. The pin test caught it -- it exists because a stale pin is what gave issue #188 a version with a first-run bug that had been fixed for months. Unrelated to this branch's change, but it fails CI on every branch until it is fixed. * fix(fleet): a NaN balance was accepted, and rejecting one was a 500 Found by re-reading the endpoint rather than from a report. JSON has no NaN or Infinity. Python's parser accepts them anyway, so {"balance": NaN} was stored verbatim. One such reading poisons every delta taken from that series -- NaN - x is NaN, and every comparison against it is False, so the clamp silently misbehaves -- the account total becomes NaN, and serialising that back out emits a bare NaN that JSON.parse rejects. A single bad reading from one client breaks the dashboard for everyone. Both float fields now refuse non-finite values. That exposed a second problem underneath: FastAPI's 422 body echoes the offending input, so the REJECTION could not be serialised either and the client got a 500 for what is squarely a bad request. A RequestValidationError handler now renders non-finite floats as their names -- keeping the message diagnostic rather than dropping the field -- and fixes the whole class instead of the one endpoint that takes a float today. My first version of that handler broke every OTHER validation error: a custom validator's error carries the raised ValueError OBJECT in ctx, which is not serialisable, and skipping jsonable_encoder turned each one into a 500. Caught by the date tests, and now pinned by its own regression test. Also: the skipped list is deduplicated. A client pushing 400 days of a platform this server does not know got the same name back 400 times -- a response that grows with the request, echoing client-supplied strings, and saying nothing the set does not. A negative control showed the sanitiser's bool guard was dead (bool subclasses int, not float), so it is gone along with the comment that justified it incorrectly. Refs: CashPilot-Desktop-xjr * perf(fleet): import in one transaction, and roll back when it fails A thousand-reading import committed a thousand times. Every commit is an fsync that takes SQLite's write lock, so one import serialised a thousand disk syncs against this server's own collector and request latency tracked sync cost rather than row count. upsert_earnings_many does the same upsert with executemany and one commit. Only half the reported cause was real, and the distinction matters for anyone reading this later: _get_db hands out a borrowed handle on a SHARED per-loop connection whose close() is a documented no-op, so the loop was never opening and closing a thousand connections. It was committing a thousand times. Batching then introduced a bug of its own, which the tests caught: the connection is shared and outlives the request, so a failed batch left an abandoned transaction holding the write lock and the NEXT write blocked for twelve seconds before timing out. It now rolls back. Two more from the same review: * docs/fleet.md gains the Authorization header and every status the endpoint can answer with. A reader integrating against that page alone could not previously construct a valid request. * Two assertions were tuple expressions -- `mock.assert_not_awaited(), "why"` builds a tuple and discards the message, so a red build showed the mock's generic text instead of the reason. Fixed here and in test_optional_runtime.py. The review said ruff's B018 catches that pattern when bugbear is enabled. Bugbear IS enabled here and B018 is not ignored, and ruff 0.15.14 passes it clean -- checked against a minimal probe rather than assumed. So nothing in CI would have caught a recurrence, and there is now an AST-based test that does. Structural, not a string search, because a string search would match the pattern inside its own docstring. One negative control PASSED, which meant the test was wrong: the transactionality test raised while BUILDING the rows, before any SQL ran, so it proved only that the row build validates first and it passed against a writer that committed after every row. It now fails inside the statement, and both it and the wedged-connection test fail under their controls. Reported by CodeRabbit on PR #256.
1 parent 3c89898 commit 55196e8

5 files changed

Lines changed: 1046 additions & 2 deletions

File tree

app/database.py

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,12 @@
88
from __future__ import annotations
99

1010
import asyncio
11+
import contextlib
1112
import json
1213
import logging
1314
import math
1415
import os
16+
from collections.abc import Mapping, Sequence
1517
from datetime import UTC, datetime, timedelta
1618
from pathlib import Path
1719
from typing import Any
@@ -921,6 +923,78 @@ async def upsert_earnings(
921923
await db.close()
922924

923925

926+
async def upsert_earnings_many(readings: Sequence[Mapping[str, Any]]) -> int:
927+
"""Upsert many readings in ONE transaction. Returns how many were written.
928+
929+
Same statement and same conflict rules as :func:`upsert_earnings` — this is
930+
purely about how often the work is committed.
931+
932+
WHY IT EXISTS
933+
-------------
934+
A client importing its pre-pairing history sends up to a thousand readings
935+
per request, and calling ``upsert_earnings`` in a loop commits once per row.
936+
Every commit is an fsync, and every one of them takes SQLite's write lock —
937+
so a single import serialised a thousand disk syncs against the server's own
938+
collector, and request latency tracked disk sync cost rather than row count.
939+
940+
(The connection is NOT the problem, despite appearances: ``_get_db`` hands
941+
out a borrowed handle on a shared per-loop connection whose ``close()`` is a
942+
documented no-op, so the loop was never opening and closing a thousand
943+
connections. It was committing a thousand times.)
944+
945+
ONE TRANSACTION ALSO MEANS ALL-OR-NOTHING, which is the behaviour to want
946+
here: a failure part-way through leaves the caller's history exactly as it
947+
was rather than half-applied, and the import is idempotent so retrying costs
948+
a round trip.
949+
"""
950+
rows = [
951+
(
952+
r["platform"],
953+
float(r["balance"]),
954+
(r.get("currency") or "USD"),
955+
r.get("date") or datetime.now(UTC).strftime("%Y-%m-%d"),
956+
r.get("fx_rate_usd"),
957+
(r.get("source") or "server"),
958+
)
959+
for r in readings
960+
]
961+
if not rows:
962+
# No statement, no commit. An empty import is a normal case, and taking
963+
# the write lock to do nothing would still block the collector.
964+
return 0
965+
966+
db = await _get_db()
967+
try:
968+
try:
969+
await db.executemany(
970+
"""
971+
INSERT INTO earnings (platform, balance, currency, date, fx_rate_usd, source)
972+
VALUES (?, ?, ?, ?, ?, ?)
973+
ON CONFLICT(platform, source, date) DO UPDATE SET
974+
balance = excluded.balance,
975+
currency = excluded.currency,
976+
fx_rate_usd = COALESCE(excluded.fx_rate_usd, earnings.fx_rate_usd),
977+
created_at = datetime('now')
978+
WHERE earnings.balance != excluded.balance
979+
OR earnings.fx_rate_usd IS NULL
980+
""",
981+
rows,
982+
)
983+
await db.commit()
984+
except Exception:
985+
# ROLL BACK, do not merely propagate. The connection is SHARED and
986+
# outlives the request, so an abandoned transaction keeps SQLite's
987+
# write lock and every later write on this loop -- including this
988+
# server's own collector -- blocks until it times out. Measured: the
989+
# next write took twelve seconds before this was added.
990+
with contextlib.suppress(Exception):
991+
await db.rollback()
992+
raise
993+
finally:
994+
await db.close()
995+
return len(rows)
996+
997+
924998
async def get_earnings_summary() -> list[dict[str, Any]]:
925999
"""Return the latest balance for each platform."""
9261000
db = await _get_db()

app/main.py

Lines changed: 182 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
import hmac
1212
import json
1313
import logging
14+
import math
1415
import os
1516
import re
1617
import secrets
@@ -23,9 +24,11 @@
2324
from apscheduler.events import EVENT_JOB_ERROR, EVENT_JOB_MISSED
2425
from apscheduler.schedulers.asyncio import AsyncIOScheduler
2526
from fastapi import FastAPI, HTTPException, Request
27+
from fastapi.encoders import jsonable_encoder
28+
from fastapi.exceptions import RequestValidationError
2629
from fastapi.responses import JSONResponse, PlainTextResponse
2730
from fastapi.staticfiles import StaticFiles
28-
from pydantic import BaseModel
31+
from pydantic import BaseModel, Field, field_validator
2932
from starlette.middleware.base import BaseHTTPMiddleware
3033

3134
from app import (
@@ -984,6 +987,51 @@ async def dispatch(self, request, call_next):
984987

985988
app.add_middleware(_SecurityHeadersMiddleware)
986989

990+
991+
@app.exception_handler(RequestValidationError)
992+
async def _validation_error_handler(request: Request, exc: RequestValidationError) -> JSONResponse:
993+
"""FastAPI's 422 body echoes the offending input, and some inputs cannot be
994+
encoded — so the rejection itself became a 500.
995+
996+
JSON has no ``NaN`` or ``Infinity``. Python's parser accepts them anyway, so
997+
a client can put one in a float field; pydantic correctly rejects it, and
998+
then the default handler tries to serialise ``{"input": nan}`` and Starlette's
999+
encoder raises "Out of range float values are not JSON compliant". The client
1000+
gets an opaque 500 for what is squarely a bad request, and the log fills with
1001+
a traceback that names the encoder rather than the cause.
1002+
1003+
Non-finite floats are therefore rendered as their names. That keeps the
1004+
message diagnostic ("we saw NaN") instead of dropping the field, and it fixes
1005+
the whole class rather than the one endpoint that happens to take a float
1006+
today. Every other error is passed through byte-for-byte, so the response
1007+
shape callers already parse is unchanged.
1008+
"""
1009+
# jsonable_encoder FIRST, exactly as FastAPI's own handler does: a custom
1010+
# validator's error carries the raised ValueError OBJECT in ctx, which is not
1011+
# serialisable either. Encoding then sanitising fixes both without changing
1012+
# the body for any error that was already fine.
1013+
return JSONResponse(status_code=422, content={"detail": _json_safe(jsonable_encoder(exc.errors()))})
1014+
1015+
1016+
def _json_safe(value: Any) -> Any:
1017+
"""Recursively replace values the JSON encoder refuses — currently only
1018+
non-finite floats.
1019+
1020+
Deliberately narrow. ``bool`` needs no special case: it subclasses ``int``,
1021+
not ``float``, so the check below never sees it. An earlier version guarded
1022+
for it anyway, with a comment that was simply wrong; a negative control
1023+
showed removing the guard changed nothing, so it went.
1024+
(``test_booleans_survive_as_booleans`` still pins the guarantee.)
1025+
"""
1026+
if isinstance(value, dict):
1027+
return {k: _json_safe(v) for k, v in value.items()}
1028+
if isinstance(value, (list, tuple)):
1029+
return [_json_safe(v) for v in value]
1030+
if isinstance(value, float) and not math.isfinite(value):
1031+
return repr(value) # "nan", "inf", "-inf"
1032+
return value
1033+
1034+
9871035
# Static files
9881036
app.mount("/static", StaticFiles(directory="app/static"), name="static")
9891037

@@ -3713,6 +3761,139 @@ async def _earnings_for_worker(body: WorkerHeartbeat, days: int = 30) -> dict[st
37133761
}
37143762

37153763

3764+
#: The exact shape ``upsert_earnings`` writes and both delta readers ORDER BY.
3765+
#: A date in any other shape would sort into the wrong place in its own series,
3766+
#: so the readings either side of it difference against the wrong neighbour --
3767+
#: silently, and only for the client that sent it.
3768+
_ISO_DATE = re.compile(r"^\d{4}-\d{2}-\d{2}$")
3769+
3770+
3771+
class EarningsReading(BaseModel):
3772+
"""One historical balance reading from a paired client."""
3773+
3774+
slug: str
3775+
#: allow_inf_nan=False because JSON's `NaN` and `Infinity` -- which Python's
3776+
#: parser accepts even though the spec does not -- are otherwise stored
3777+
#: verbatim. One NaN balance poisons every delta taken from that series
3778+
#: (NaN - x is NaN, and every comparison against it is False, so the clamp
3779+
#: silently misbehaves), the account total becomes NaN, and serialising that
3780+
#: back out emits a bare `NaN` that JSON.parse rejects. So a single bad
3781+
#: reading from one client breaks the dashboard for everyone.
3782+
balance: float = Field(allow_inf_nan=False)
3783+
date: str
3784+
currency: str = "USD"
3785+
fx_rate_usd: float | None = Field(default=None, allow_inf_nan=False)
3786+
3787+
@field_validator("date")
3788+
@classmethod
3789+
def _iso_date(cls, value: str) -> str:
3790+
value = (value or "").strip()
3791+
if not _ISO_DATE.match(value):
3792+
raise ValueError("date must be YYYY-MM-DD")
3793+
# Reject 2026-02-30 and friends: the pattern above only proves shape.
3794+
try:
3795+
datetime.strptime(value, "%Y-%m-%d") # noqa: DTZ007 -- a calendar date, not an instant
3796+
except ValueError as exc:
3797+
raise ValueError("date must be a real calendar date") from exc
3798+
return value
3799+
3800+
3801+
class EarningsImport(BaseModel):
3802+
"""A client pushing the history it collected before it was paired.
3803+
3804+
Deliberately carries NO source field. The source is taken from the
3805+
AUTHENTICATED worker, never from the body -- otherwise any enrolled client
3806+
could write into the 'server' series, or into another machine's, and
3807+
overwrite readings it never took.
3808+
"""
3809+
3810+
client_id: str
3811+
#: Bounded so one authenticated client cannot hand the server an
3812+
#: arbitrarily large body to parse and then write row by row. 2000 is
3813+
#: comfortably above a real import -- the server keeps 400 days, and a
3814+
#: client chunks anything larger -- while staying a body the process can
3815+
#: hold. An unbounded list here is a denial of service that needs only one
3816+
#: compromised worker.
3817+
readings: list[EarningsReading] = Field(default_factory=list, max_length=2000)
3818+
3819+
3820+
@app.post("/api/workers/earnings-import")
3821+
async def api_worker_earnings_import(request: Request, body: EarningsImport) -> dict[str, Any]:
3822+
"""Accept a paired client's historical earnings under its own source.
3823+
3824+
This exists because the server and a Desktop can both have been reading the
3825+
SAME provider account. Their readings must not be merged into one series:
3826+
earnings are clamped deltas between consecutive readings, so interleaving two
3827+
samplers makes every drop clamp to zero and the total comes out
3828+
systematically understated. Each client's rows therefore land under its own
3829+
``source`` and are differenced separately.
3830+
3831+
Idempotent by construction: the (platform, source, date) unique index means a
3832+
re-pair or a retried import UPDATES a day rather than adding a second reading
3833+
for it, which would difference against itself and read as zero.
3834+
"""
3835+
cid = (body.client_id or "").strip()
3836+
if not cid:
3837+
raise HTTPException(status_code=400, detail="client_id required")
3838+
3839+
state = await _authenticate_worker_heartbeat(request, cid)
3840+
# Only a CONFIRMED worker may import. "enroll" and "reissue" both mean the
3841+
# caller presented the SHARED key, which every worker holds -- accepting it
3842+
# here would let anyone with that token write a history for any client_id
3843+
# they cared to name. A heartbeat is idempotent status; this is durable
3844+
# money data, so it gets the stricter bar.
3845+
if state != "ok":
3846+
raise HTTPException(
3847+
status_code=403,
3848+
detail=(
3849+
"Importing earnings requires this worker's own key. Send a heartbeat first "
3850+
"to complete enrollment, then retry."
3851+
),
3852+
)
3853+
3854+
known = {s["slug"] for s in catalog.get_services()}
3855+
# DISTINCT slugs, not one entry per skipped reading. A client pushing 400
3856+
# days of a platform this server does not know would otherwise get the same
3857+
# name back 400 times -- a response that grows with the request, echoing
3858+
# client-supplied strings, and tells the reader nothing the set does not.
3859+
skipped: set[str] = set()
3860+
rows: list[dict[str, Any]] = []
3861+
for reading in body.readings:
3862+
slug = (reading.slug or "").strip()
3863+
# An unknown slug is dropped rather than stored: it would create a
3864+
# platform the catalog cannot render, name or ever collect for again.
3865+
if not slug or slug not in known:
3866+
skipped.add(slug or "(blank)")
3867+
continue
3868+
rows.append(
3869+
{
3870+
"platform": slug,
3871+
"balance": float(reading.balance),
3872+
"currency": (reading.currency or "USD").upper(),
3873+
"date": reading.date,
3874+
"fx_rate_usd": reading.fx_rate_usd,
3875+
"source": cid,
3876+
}
3877+
)
3878+
3879+
# ONE transaction for the whole batch. Writing row by row committed once per
3880+
# reading, and every commit is an fsync that takes SQLite's write lock -- so
3881+
# a thousand-reading import serialised a thousand disk syncs against this
3882+
# server's own collector, and latency tracked sync cost rather than row
3883+
# count. It also makes the import all-or-nothing: a failure part-way leaves
3884+
# the client's history exactly as it was rather than half-applied, and the
3885+
# import is idempotent so retrying costs a round trip. (CodeRabbit, PR #256.)
3886+
written = await database.upsert_earnings_many(rows)
3887+
3888+
logger.info(
3889+
"Imported %d earnings reading(s) from worker '%s' (%d unknown platform(s) skipped)",
3890+
written,
3891+
cid,
3892+
len(skipped),
3893+
)
3894+
return {"status": "ok", "imported": written, "skipped": sorted(skipped), "source": cid}
3895+
3896+
37163897
@app.post("/api/workers/heartbeat")
37173898
async def api_worker_heartbeat(request: Request, body: WorkerHeartbeat) -> dict[str, Any]:
37183899
"""Receive a heartbeat from a worker. Registers or updates the worker."""

docs/fleet.md

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,69 @@ Workers use **REST HTTP** to communicate with the UI:
3434

3535
Workers must be reachable from the UI for commands. The UI must be reachable from workers for heartbeats.
3636

37+
### Importing earnings a client collected on its own
38+
39+
`POST /api/workers/earnings-import` lets a client that has been reading a
40+
provider account by itself — CashPilot Desktop, typically, before it was paired —
41+
hand that history to the UI, so the fleet view shows the complete picture rather
42+
than starting from the day of pairing.
43+
44+
```http
45+
POST /api/workers/earnings-import
46+
Authorization: Bearer <this worker's own key, not the shared enrollment key>
47+
Content-Type: application/json
48+
49+
{
50+
"client_id": "desktop-macbook",
51+
"readings": [
52+
{"slug": "honeygain", "balance": 12.4, "date": "2026-07-01", "currency": "USD"},
53+
{"slug": "mysterium", "balance": 88.0, "date": "2026-07-01", "currency": "MYST", "fx_rate_usd": 0.41}
54+
]
55+
}
56+
```
57+
58+
It answers:
59+
60+
| Status | Meaning |
61+
|---|---|
62+
| `200` | `{"status": "ok", "imported": N, "skipped": [...], "source": "<client_id>"}` |
63+
| `400` | `client_id` was missing or blank |
64+
| `401` | the key is wrong or revoked |
65+
| `403` | this worker is not fully enrolled yet — send a heartbeat first, then retry |
66+
| `422` | the body was rejected: a date that is not a real `YYYY-MM-DD` day, a non-finite `balance` or `fx_rate_usd`, or more than 2000 readings |
67+
68+
Three things about it are deliberate:
69+
70+
- **Each client's readings are stored under their own source, not merged with the
71+
server's.** Earnings are clamped deltas between consecutive readings of the same
72+
balance, so interleaving two samplers of one account makes every apparent drop
73+
clamp to zero and understates the total. Separate series are differenced
74+
separately and then summed.
75+
- **The source is taken from the authenticated worker, never from the request
76+
body.** Otherwise any enrolled client could write into another's history, or
77+
into the server's own.
78+
- **Only a fully enrolled worker may import.** A caller still presenting the
79+
shared enrollment key gets `403` with instructions to heartbeat first: every
80+
worker holds that key, and this writes durable money data.
81+
82+
Re-sending the same day updates it rather than appending, so a retried or
83+
repeated import is safe.
84+
85+
`date` must be `YYYY-MM-DD` and a real calendar day. It is not free text: both
86+
delta readers order by it, so a differently shaped date sorts into the wrong
87+
place in its own series and the readings either side then difference against the
88+
wrong neighbour — silently, and only in the earned figure. A request may carry at
89+
most **2000 readings**; send larger histories in chunks.
90+
91+
`balance` and `fx_rate_usd` must be finite. JSON has no `NaN` or `Infinity`, but
92+
Python's parser accepts them, and one `NaN` balance poisons every earned figure
93+
taken from that series — every comparison against it is false, so the clamp
94+
misbehaves silently and the account total becomes `NaN`. They are rejected with
95+
`422`.
96+
97+
Unknown slugs come back in `skipped` as a **distinct, sorted** list, so a year of
98+
one unrecognised platform is reported once rather than 400 times.
99+
37100
## Setting Up the Fleet
38101

39102
### Main server (UI + local worker)

0 commit comments

Comments
 (0)