From 0878387c2352572ecdc7b6d245c559350321eb79 Mon Sep 17 00:00:00 2001 From: Jason Antman Date: Sat, 4 Jul 2026 07:49:15 -0400 Subject: [PATCH 1/9] new feature --- .../{ => completed}/mcu-heartbeat-resilience.md | 0 docs/features/esb-support.md | 15 +++++++++++++++ 2 files changed, 15 insertions(+) rename docs/features/{ => completed}/mcu-heartbeat-resilience.md (100%) create mode 100644 docs/features/esb-support.md diff --git a/docs/features/mcu-heartbeat-resilience.md b/docs/features/completed/mcu-heartbeat-resilience.md similarity index 100% rename from docs/features/mcu-heartbeat-resilience.md rename to docs/features/completed/mcu-heartbeat-resilience.md diff --git a/docs/features/esb-support.md b/docs/features/esb-support.md new file mode 100644 index 0000000..1a4d01e --- /dev/null +++ b/docs/features/esb-support.md @@ -0,0 +1,15 @@ +# Feature Template + +You must read, understand, and follow all instructions in `./README.md` when planning and implementing this feature. + +## Overview + +Goal: implement the required minimal changes in this application to support https://github.com/DecaturMakers/equipment-status-board/issues/10. + +At a high level, this means: + +1. If we don't already have one, a simple API for retrieving a list of all machines and their current status. +2. Ability to fire a webhook (non-blocking/async or from a background worker) when a machine's status is changed (user login/logout, unauthorized RFID fob, oops/lockout/clear), using a webhook destination configured via environment variable. The webhook should include information on the currently-logged-in user if there is one. This should only fire on meaningful status changes, not on every update from the MCU. +3. If we don't already have them, API endpoints to oops a machine, put it in maintenance lockout, or clear oops/lockout. + +Our approach is to minimize change in this application, and let ESB do the heavy lifting; we just need to send events to it via webhook and give it the appropriate API endpoints to call. From 4d8f1b427610822085f4f31438d5017f9e3976bb Mon Sep 17 00:00:00 2001 From: Jason Antman Date: Sat, 4 Jul 2026 08:40:34 -0400 Subject: [PATCH 2/9] ESB Support - 1: add machine status representation and GET /api/machines Implements Milestone 1 of the ESB support feature: the read-only status API and the shared status representation that the status-change webhook (Milestone 2) will reuse. - Add Machine.status (derived: locked_out/oops/in_use/idle/unknown) and Machine.status_dict (name, display_name, status, relay, oops, locked_out, current_user, last_checkin, last_update) as the single source of truth for both the list API and the upcoming webhook payload. - Add GET /api/machines returning {"machines": [status_dict, ...]} sorted by name, with MachineStatus / MachineStatusUser / MachinesListResponse pydantic schemas documented via quart_schema. - Unit tests for status/status_dict across all states (with and without a logged-in user) and for the endpoint reflecting live machine state. Requirement #3 (oops/lockout/clear control endpoints) already exists in views/machine.py and is well covered, so no code change was needed there. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/features/esb-support.md | 124 +++++++++++++++++++++++++++++++ src/dm_mac/models/api_schemas.py | 51 +++++++++++++ src/dm_mac/models/machine.py | 48 ++++++++++++ src/dm_mac/views/api.py | 20 +++++ tests/models/test_machine.py | 95 +++++++++++++++++++++++ tests/views/test_api.py | 43 +++++++++++ 6 files changed, 381 insertions(+) diff --git a/docs/features/esb-support.md b/docs/features/esb-support.md index 1a4d01e..ecc2af8 100644 --- a/docs/features/esb-support.md +++ b/docs/features/esb-support.md @@ -13,3 +13,127 @@ At a high level, this means: 3. If we don't already have them, API endpoints to oops a machine, put it in maintenance lockout, or clear oops/lockout. Our approach is to minimize change in this application, and let ESB do the heavy lifting; we just need to send events to it via webhook and give it the appropriate API endpoints to call. + +## Implementation Plan + +Commit-message prefix for this feature: ``ESB Support - {Milestone}.{Task}``. + +### Findings / Scope + +Mapping the three high-level requirements onto the current codebase: + +1. **Machine list/status API** — *does not exist as JSON.* Today status is only + available via the Prometheus ``/metrics`` endpoint and the Slack ``status`` + command. We will add a small read-only JSON endpoint. +2. **Status-change webhook** — *does not exist.* The meaningful status changes + already have well-defined chokepoints in ``models/machine.py`` where Slack + notifications fire (login, logout, unauthorized/unknown fob, override login, + oops, un-oops, lockout, unlock, reboot). We will fire the webhook from those + same sites so it only fires on meaningful changes, never on ordinary MCU + heartbeats. +3. **Oops / lockout / clear API endpoints** — *already exist* in + ``views/machine.py``: ``POST``/``DELETE /api/machine/oops/`` and + ``POST``/``DELETE /api/machine/locked_out/``. ESB "clear" = ``DELETE`` + on whichever of the two is set. No new control endpoints are required; we + only verify and document them. + +The ESB issue also mentions storing 100–500 recent activity events per machine. +Per the "let ESB do the heavy lifting" directive, **ESB stores that history from +the webhook events**; MAC does not add history storage. + +### Design Decisions (confirmed with maintainer) + +* **Webhook destination:** ``STATUS_WEBHOOK_URL`` environment variable. When + unset, webhook firing is disabled entirely (mirrors how Slack is disabled + without its tokens). +* **Authentication:** none. The POST is sent with no auth header. +* **Delivery:** fire-and-forget (``asyncio.create_task``, like Slack) so the MCU + response is never blocked, with a bounded **retry-with-backoff** in the + background task and a per-attempt timeout. Failures are logged after retries + are exhausted; ESB can reconcile via the status API. + +### Shared status representation + +A single ``status_dict`` (and derived ``status`` string) will be used by **both** +the list API and the webhook payload so they never drift: + +```json +{ + "name": "planer", + "display_name": "Planer", + "status": "in_use", // one of: idle | in_use | oops | locked_out | unknown + "relay": true, + "oops": false, + "locked_out": false, + "current_user": { "account_id": "123", "full_name": "Jane Doe" }, // or null + "last_checkin": 1720000000.0, // epoch seconds, or null if never + "last_update": 1720000000.0 // epoch seconds, or null if never +} +``` + +The webhook payload is this dict plus an ``event`` field and a firing +``timestamp``. Event values: ``login``, ``logout``, ``unauthorized``, +``unknown_fob``, ``override_login``, ``oops``, ``unoops``, ``lockout``, +``unlock``, ``reboot``. + +### Milestone 1 — ESB-facing status & control API — ✅ COMPLETE + +Added ``Machine.status`` and ``Machine.status_dict`` (shared representation), +the ``GET /api/machines`` endpoint with ``MachineStatus`` / ``MachinesListResponse`` +schemas, and unit tests for both the model properties and the endpoint. Verified +the existing oops/lockout endpoints already satisfy requirement #3 and are +well-covered (no code change needed). All ``nox -s tests`` pass at 97% coverage. + +* **1.1** Add a ``status`` property (derived string) and a ``status_dict`` + property on ``Machine`` (reading ``self.state``), producing the shared + representation above. ``current_user`` serializes to ``{account_id, full_name}`` + or ``null``. +* **1.2** Add ``GET /api/machines`` returning ``{"machines": [status_dict, ...]}`` + for all configured machines, sorted by name. Add ``pydantic`` response schemas + in ``models/api_schemas.py`` and document the route with ``quart_schema`` + (tagged ``Admin``). Read-only and unauthenticated, consistent with the existing + API surface. +* **1.3** Confirm the existing oops/lockout endpoints satisfy ESB's control + needs; add any missing test coverage. No behavior change expected. +* **1.4** Unit tests for ``status``/``status_dict`` and the new endpoint + (idle/in-use/oops/locked-out, with and without a current user). + +### Milestone 2 — Status-change webhook + +* **2.1** New module ``src/dm_mac/webhook.py`` with a ``WebhookNotifier`` class: + * Constructed from ``STATUS_WEBHOOK_URL``. + * ``notify(machine, event, user=None)`` builds the payload from + ``machine.status_dict`` + ``event`` + ``timestamp`` and spawns a + fire-and-forget ``asyncio.create_task`` delivery coroutine. + * Delivery coroutine POSTs JSON via ``aiohttp`` (already a dependency) with a + per-attempt timeout and retry-with-backoff (a small, bounded number of + attempts with exponential backoff); logs a single error after exhaustion. +* **2.2** In ``create_app``/``main`` (``__init__.py``), instantiate the notifier + when ``STATUS_WEBHOOK_URL`` is set and store it as + ``app.config["WEBHOOK_NOTIFIER"]`` (default ``None``), exactly like + ``SLACK_HANDLER``. +* **2.3** Fire ``notify(...)`` from the same async sites that emit Slack + messages, so webhooks track meaningful changes only: + * ``MachineState._handle_rfid_insert`` → ``login`` / ``unauthorized`` / + ``unknown_fob`` / ``override_login`` + * ``MachineState._handle_rfid_remove`` → ``logout`` + * ``MachineState._handle_reboot`` → ``reboot`` (and logout of any prior user) + * ``Machine.oops`` / ``MachineState._handle_oops`` → ``oops`` + * ``Machine.unoops`` → ``unoops``; ``Machine.lockout`` → ``lockout``; + ``Machine.unlock`` → ``unlock`` + Each site fetches the notifier via ``current_app.config.get("WEBHOOK_NOTIFIER")`` + and no-ops when ``None``. +* **2.4** Unit tests: payload shape per event, disabled-when-unset, retry/backoff + on failure (mock ``aiohttp``), and non-firing on ordinary heartbeat updates. + +### Milestone 3 — Acceptance Criteria + +* **3.1** Update documentation: ``README.md`` (env var + endpoints if listed), + ``docs/source/configuration.rst`` (add ``STATUS_WEBHOOK_URL`` to the env-var + table), ``docs/source/http-api.rst`` (document ``GET /api/machines`` and the + webhook payload/events), and ``CLAUDE.md`` (env vars + architecture notes). + Match the existing style and verbosity. +* **3.2** Ensure all new code has appropriate unit-test coverage. +* **3.3** All ``nox`` sessions pass (``tests``, ``mypy``, ``pre-commit``, + ``safety``, ``typeguard``, ``docs``). +* **3.4** Move this file from ``docs/features/`` to ``docs/features/completed/``. diff --git a/src/dm_mac/models/api_schemas.py b/src/dm_mac/models/api_schemas.py index b8f1789..1c056ec 100644 --- a/src/dm_mac/models/api_schemas.py +++ b/src/dm_mac/models/api_schemas.py @@ -55,6 +55,57 @@ class MachineUpdateResponse(BaseModel): ) +class MachineStatusUser(BaseModel): + """The user currently logged in to a machine, if any.""" + + account_id: str = Field(description="Unique account ID of the user.") + full_name: str = Field(description="Full name of the user.") + + +class MachineStatus(BaseModel): + """Current status of a single machine. + + Shared representation used by ``GET /api/machines`` and the status-change + webhook (which adds ``event`` and ``timestamp`` fields). + """ + + name: str = Field(description="Unique machine name.") + display_name: str = Field( + description="Human-friendly name (alias if configured, else name)." + ) + status: str = Field( + description="Derived status: one of 'locked_out', 'oops', 'in_use', " + "'idle', or 'unknown' (never checked in)." + ) + relay: bool = Field(description="Whether the primary relay is energized.") + oops: bool = Field(description="Whether the machine is in Oops state.") + locked_out: bool = Field( + description="Whether the machine is locked out for maintenance." + ) + current_user: Optional[MachineStatusUser] = Field( + default=None, + description="The logged-in user, or null if no user is logged in.", + ) + last_checkin: Optional[float] = Field( + default=None, + description="Epoch seconds of the machine's last check-in, or null " + "if it has never checked in.", + ) + last_update: Optional[float] = Field( + default=None, + description="Epoch seconds of the machine's last meaningful state " + "change, or null if none.", + ) + + +class MachinesListResponse(BaseModel): + """Response body for GET /api/machines (200).""" + + machines: List[MachineStatus] = Field( + description="Status of all configured machines, sorted by name." + ) + + class SuccessResponse(BaseModel): """Generic success response.""" diff --git a/src/dm_mac/models/machine.py b/src/dm_mac/models/machine.py index 6131e0b..7d73a36 100644 --- a/src/dm_mac/models/machine.py +++ b/src/dm_mac/models/machine.py @@ -320,6 +320,54 @@ def display_name(self) -> str: """Return the display name for this machine (alias if present, else name).""" return self.alias if self.alias else self.name + @property + def status(self) -> str: + """Return a single-word derived status string for this machine. + + One of ``locked_out``, ``oops``, ``in_use``, ``idle``, or ``unknown`` + (the latter only when the machine has never checked in and is not in + any other state). Used by the ``GET /api/machines`` endpoint and the + status-change webhook so both share a single source of truth. + """ + state: "MachineState" = self.state + if state.is_locked_out: + return "locked_out" + if state.is_oopsed: + return "oops" + if state.relay_desired_state: + return "in_use" + if state.last_checkin is None: + return "unknown" + return "idle" + + @property + def status_dict(self) -> Dict[str, Any]: + """Return the shared ESB status representation for this machine. + + Consumed by both ``GET /api/machines`` and the status-change webhook + (which adds ``event`` and ``timestamp`` fields). ``current_user`` is + ``{"account_id": ..., "full_name": ...}`` when a user is logged in, or + ``None`` otherwise. + """ + state: "MachineState" = self.state + current_user: Optional[Dict[str, str]] = None + if state.current_user is not None: + current_user = { + "account_id": state.current_user.account_id, + "full_name": state.current_user.full_name, + } + return { + "name": self.name, + "display_name": self.display_name, + "status": self.status, + "relay": state.relay_desired_state, + "oops": state.is_oopsed, + "locked_out": state.is_locked_out, + "current_user": current_user, + "last_checkin": state.last_checkin, + "last_update": state.last_update, + } + @property def as_dict(self) -> Dict[str, Any]: """Return a dict representation of this machine.""" diff --git a/src/dm_mac/views/api.py b/src/dm_mac/views/api.py index 6626ddb..3980421 100644 --- a/src/dm_mac/views/api.py +++ b/src/dm_mac/views/api.py @@ -13,7 +13,9 @@ from dm_mac.models.api_schemas import ApiIndexResponse from dm_mac.models.api_schemas import ErrorResponse +from dm_mac.models.api_schemas import MachinesListResponse from dm_mac.models.api_schemas import ReloadUsersResponse +from dm_mac.models.machine import MachinesConfig from dm_mac.models.users import UsersConfig logger: Logger = getLogger(__name__) @@ -32,6 +34,24 @@ async def index() -> Tuple[Response, int]: return jsonify({"message": "Nothing to see here..."}), 200 +@api.route("/machines") +@tag(["Admin"]) +@document_response(MachinesListResponse, 200) +async def machines() -> Tuple[Response, int]: + """List all machines and their current status. + + Read-only endpoint returning the status of every configured machine, + sorted by name. Intended for external consumers (e.g. the Equipment + Status Board) to poll or reconcile machine state. + """ + mconf: MachinesConfig = current_app.config["MACHINES"] # noqa + machine_list = [ + mconf.machines_by_name[name].status_dict + for name in sorted(mconf.machines_by_name) + ] + return jsonify({"machines": machine_list}), 200 + + @api.route("/reload-users", methods=["POST"]) @tag(["Admin"]) @document_response(ReloadUsersResponse, 200) diff --git a/tests/models/test_machine.py b/tests/models/test_machine.py index 751b255..b7857e9 100644 --- a/tests/models/test_machine.py +++ b/tests/models/test_machine.py @@ -6,6 +6,7 @@ from pathlib import Path from typing import Any from typing import Dict +from unittest.mock import Mock from unittest.mock import call from unittest.mock import patch @@ -185,6 +186,100 @@ def test_display_name_with_alias(self) -> None: assert cls.display_name == "My Machine" +class TestMachineStatus: + """Tests for Machine.status and Machine.status_dict.""" + + def _machine(self, **state_attrs: Any) -> Machine: + """Build a Machine with a mocked state and the given state attributes.""" + with patch(f"{pbm}.MachineState", autospec=True): + mach: Machine = Machine( + name="mName", + authorizations_or=["Foo"], + alias="My Machine", + ) + # Sensible defaults; overridden by state_attrs. + defaults: Dict[str, Any] = { + "is_locked_out": False, + "is_oopsed": False, + "relay_desired_state": False, + "last_checkin": 123.0, + "last_update": 456.0, + "current_user": None, + } + defaults.update(state_attrs) + for k, v in defaults.items(): + setattr(mach.state, k, v) + return mach + + def test_status_locked_out(self) -> None: + """locked_out takes precedence over everything else.""" + mach = self._machine( + is_locked_out=True, is_oopsed=True, relay_desired_state=True + ) + assert mach.status == "locked_out" + + def test_status_oops(self) -> None: + """oops takes precedence over in_use/idle.""" + mach = self._machine(is_oopsed=True, relay_desired_state=True) + assert mach.status == "oops" + + def test_status_in_use(self) -> None: + """relay energized with no oops/lockout is in_use.""" + mach = self._machine(relay_desired_state=True) + assert mach.status == "in_use" + + def test_status_idle(self) -> None: + """checked-in machine with relay off is idle.""" + mach = self._machine(relay_desired_state=False, last_checkin=123.0) + assert mach.status == "idle" + + def test_status_unknown(self) -> None: + """never-checked-in machine is unknown.""" + mach = self._machine(relay_desired_state=False, last_checkin=None) + assert mach.status == "unknown" + + def test_status_dict_no_user(self) -> None: + """status_dict with no logged-in user.""" + mach = self._machine( + relay_desired_state=False, + last_checkin=123.0, + last_update=456.0, + current_user=None, + ) + assert mach.status_dict == { + "name": "mName", + "display_name": "My Machine", + "status": "idle", + "relay": False, + "oops": False, + "locked_out": False, + "current_user": None, + "last_checkin": 123.0, + "last_update": 456.0, + } + + def test_status_dict_with_user(self) -> None: + """status_dict serializes the logged-in user.""" + user = Mock(account_id="42", full_name="Jane Doe") + mach = self._machine( + relay_desired_state=True, + last_checkin=123.0, + last_update=456.0, + current_user=user, + ) + assert mach.status_dict == { + "name": "mName", + "display_name": "My Machine", + "status": "in_use", + "relay": True, + "oops": False, + "locked_out": False, + "current_user": {"account_id": "42", "full_name": "Jane Doe"}, + "last_checkin": 123.0, + "last_update": 456.0, + } + + class TestMachinesConfigGetMachine: """Tests for MachinesConfig.get_machine method.""" diff --git a/tests/views/test_api.py b/tests/views/test_api.py index a1bdbc0..2be06e0 100644 --- a/tests/views/test_api.py +++ b/tests/views/test_api.py @@ -29,6 +29,49 @@ async def test_index_response(self, tmp_path: Path) -> None: assert response.headers["Content-Type"] == "application/json" +class TestMachines: + """Tests for the GET /api/machines endpoint.""" + + async def test_all_machines_listed_sorted(self, tmp_path: Path) -> None: + """All configured machines are returned, sorted by name.""" + app: Quart + client: TestClientProtocol + app, client = app_and_client(tmp_path) + response: Response = await client.get("/api/machines") + assert response.status_code == 200 + assert response.headers["Content-Type"] == "application/json" + data = await response.json + names = [m["name"] for m in data["machines"]] + assert names == sorted(names) + assert set(names) == set(app.config["MACHINES"].machines_by_name.keys()) + + async def test_status_reflects_state(self, tmp_path: Path) -> None: + """Endpoint reflects live machine state (in_use/oops/locked_out/user).""" + app, client = app_and_client(tmp_path) + mconf = app.config["MACHINES"] + # in_use with a logged-in user + mm = mconf.machines_by_name["metal-mill"] + mm.state.relay_desired_state = True + mm.state.last_checkin = 100.0 + mm.state.current_user = app.config["USERS"].users_by_fob["8114346998"] + # oopsed + mconf.machines_by_name["hammer"].state.is_oopsed = True + # locked out + mconf.machines_by_name["restrictive-lathe"].state.is_locked_out = True + response: Response = await client.get("/api/machines") + by_name = {m["name"]: m for m in (await response.json)["machines"]} + assert by_name["metal-mill"]["status"] == "in_use" + assert by_name["metal-mill"]["relay"] is True + assert by_name["metal-mill"]["current_user"] == { + "account_id": "1", + "full_name": "Ashley Williams", + } + assert by_name["hammer"]["status"] == "oops" + assert by_name["hammer"]["oops"] is True + assert by_name["restrictive-lathe"]["status"] == "locked_out" + assert by_name["restrictive-lathe"]["locked_out"] is True + + @freeze_time("2023-07-16 03:14:08", tz_offset=0) class TestReloadUsers: """Tests for reloading users.""" From 44adfcd1ada53d75b9d274883af15a58412d2158 Mon Sep 17 00:00:00 2001 From: Jason Antman Date: Sat, 4 Jul 2026 08:59:53 -0400 Subject: [PATCH 3/9] ESB Support - 2: fire status-change webhooks on meaningful machine events Implements Milestone 2: a fire-and-forget webhook that notifies an external consumer (the Equipment Status Board) of meaningful machine status changes, never on ordinary MCU heartbeats. - Add src/dm_mac/webhook.py with WebhookNotifier: enabled by STATUS_WEBHOOK_URL (disabled/None when unset), builds a payload from Machine.status_dict plus event/timestamp/user, and delivers it via a fire-and-forget asyncio task that POSTs with aiohttp using a per-attempt timeout and bounded exponential-backoff retry. The acting user is carried in a distinct `user` field (the event actor, e.g. who logged out), separate from status_dict's `current_user`. - Wire the notifier into create_app as WEBHOOK_NOTIFIER, mirroring SLACK_HANDLER. - Fire notify() from the meaningful status-change sites in models/machine.py: login, logout, unauthorized, unknown_fob, override_login, oops, unoops, lockout, unlock, reboot, plus the always-enabled RFID-tracking path. - Resolve the notifier via current_app in request-context (MCU/API) paths and via the Slack handler's app (slack.quart) in the Slack-command path, which runs without a request context. - Tests for the notifier (payload shape, retry/backoff, HTTP-error retry, give-up-and-log, from_env) and for the wiring (each event fires, heartbeats do not, Slack path fires, disabled notifier is a no-op). Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/features/esb-support.md | 17 ++- src/dm_mac/__init__.py | 2 + src/dm_mac/models/machine.py | 53 ++++++++ src/dm_mac/webhook.py | 127 ++++++++++++++++++ tests/test_webhook.py | 199 +++++++++++++++++++++++++++++ tests/views/test_webhook_wiring.py | 158 +++++++++++++++++++++++ 6 files changed, 555 insertions(+), 1 deletion(-) create mode 100644 src/dm_mac/webhook.py create mode 100644 tests/test_webhook.py create mode 100644 tests/views/test_webhook_wiring.py diff --git a/docs/features/esb-support.md b/docs/features/esb-support.md index ecc2af8..889776a 100644 --- a/docs/features/esb-support.md +++ b/docs/features/esb-support.md @@ -98,7 +98,22 @@ well-covered (no code change needed). All ``nox -s tests`` pass at 97% coverage. * **1.4** Unit tests for ``status``/``status_dict`` and the new endpoint (idle/in-use/oops/locked-out, with and without a current user). -### Milestone 2 — Status-change webhook +### Milestone 2 — Status-change webhook — ✅ COMPLETE + +Added ``src/dm_mac/webhook.py`` (``WebhookNotifier``: fire-and-forget delivery +via ``aiohttp`` with bounded exponential-backoff retry, enabled by +``STATUS_WEBHOOK_URL``), wired it into ``create_app`` as ``WEBHOOK_NOTIFIER``, +and fired ``notify(...)`` from the meaningful status-change sites in +``models/machine.py`` (login / logout / unauthorized / unknown_fob / +override_login / oops / unoops / lockout / unlock / reboot), including the +always-enabled RFID-tracking path. The notifier is resolved via +``current_app`` in the request-context (MCU/API) paths and via the passed +Slack handler's app (``slack.quart``) in the Slack-command path, which runs +without a request context. Payloads reuse ``Machine.status_dict`` plus +``event``, ``timestamp``, and a distinct ``user`` (event actor) field. Unit +tests cover the notifier (payload/retry/backoff/disabled) and the wiring +(each event fires; heartbeats do not; Slack path; disabled no-op). All +``nox -s tests`` and ``nox -s mypy`` pass; ``webhook.py`` at 100% coverage. * **2.1** New module ``src/dm_mac/webhook.py`` with a ``WebhookNotifier`` class: * Constructed from ``STATUS_WEBHOOK_URL``. diff --git a/src/dm_mac/__init__.py b/src/dm_mac/__init__.py index 4424c2f..d9f2a9e 100644 --- a/src/dm_mac/__init__.py +++ b/src/dm_mac/__init__.py @@ -24,6 +24,7 @@ from dm_mac.views.api import api from dm_mac.views.machine import machineapi from dm_mac.views.prometheus import prometheus_route +from dm_mac.webhook import WebhookNotifier logger: logging.Logger = logging.getLogger() logging.basicConfig( @@ -99,6 +100,7 @@ def create_app() -> Quart: app.config.update({"USERS": UsersConfig()}) app.config.update({"START_TIME": time()}) app.config.update({"SLACK_HANDLER": None}) + app.config.update({"WEBHOOK_NOTIFIER": WebhookNotifier.from_env()}) app.config.update({"FLEET_TIMEOUT_TRACKER": FleetTimeoutTracker()}) app.register_blueprint(api) app.add_url_rule("/metrics", view_func=prometheus_route) diff --git a/src/dm_mac/models/machine.py b/src/dm_mac/models/machine.py index 7d73a36..ff5f08b 100644 --- a/src/dm_mac/models/machine.py +++ b/src/dm_mac/models/machine.py @@ -31,6 +31,7 @@ if TYPE_CHECKING: # pragma: no cover from dm_mac.slack_handler import SlackHandler + from dm_mac.webhook import WebhookNotifier logger: Logger = getLogger(__name__) @@ -274,6 +275,7 @@ async def lockout(self, slack: Optional["SlackHandler"] = None) -> None: if not slack: slack = current_app.config.get("SLACK_HANDLER") source = "API" + self.state._notify_status_webhook("lockout", slack=slack) if not slack: # Slack integration is not enabled return @@ -286,6 +288,7 @@ async def unlock(self, slack: Optional["SlackHandler"] = None) -> None: if not slack: slack = current_app.config.get("SLACK_HANDLER") source = "API" + self.state._notify_status_webhook("unlock", slack=slack) if not slack: # Slack integration is not enabled return @@ -298,6 +301,7 @@ async def oops(self, slack: Optional["SlackHandler"] = None) -> None: if not slack: slack = current_app.config.get("SLACK_HANDLER") source = "API" + self.state._notify_status_webhook("oops", slack=slack) if not slack: # Slack integration is not enabled return @@ -310,6 +314,7 @@ async def unoops(self, slack: Optional["SlackHandler"] = None) -> None: if not slack: slack = current_app.config.get("SLACK_HANDLER") source = "API" + self.state._notify_status_webhook("unoops", slack=slack) if not slack: # Slack integration is not enabled return @@ -761,6 +766,39 @@ def _notify_fleet_save_timeout(self) -> None: "notification" ) + def _notify_status_webhook( + self, + event: str, + user: Optional[User] = None, + slack: Optional["SlackHandler"] = None, + ) -> None: + """Fire a status-change webhook, if a notifier is configured. + + No-op when ``WEBHOOK_NOTIFIER`` is unset (``STATUS_WEBHOOK_URL`` not + configured). Called only from meaningful status-change code paths so + the webhook never fires on ordinary MCU heartbeats. ``user`` is the + actor involved in the event (e.g. the user who just logged out), which + may differ from the machine's post-event ``current_user``. + + The notifier lives in the Quart app config. MCU-update and API request + handlers run within a request context so it is reachable via + :data:`current_app`; the Slack command handlers do **not** run within a + request context, so when a :class:`SlackHandler` is passed we fall back + to its held app reference (``slack.quart``). + """ + notifier: Optional["WebhookNotifier"] = None + try: + notifier = current_app.config.get("WEBHOOK_NOTIFIER") + except RuntimeError: + # No application context (Slack command path, or a unit test + # invoking a state method directly). Use the app held by the + # Slack handler if we have one; otherwise there is nothing to do. + if slack is not None: + notifier = slack.quart.config.get("WEBHOOK_NOTIFIER") + if notifier is None: + return + notifier.notify(self.machine, event, user=user) + def _load_from_cache(self) -> None: """Load machine state cache from disk.""" if not os.path.exists(self._state_path): @@ -789,6 +827,7 @@ async def _handle_reboot(self) -> None: self.machine.display_name, ) # locking handled in update() + prior_user: Optional[User] = self.current_user self.current_user = None self.is_override_login = False # Restore always-enabled state if applicable @@ -803,6 +842,7 @@ async def _handle_reboot(self) -> None: self.status_led_rgb = (0.0, 0.0, 0.0) self.status_led_brightness = 0.0 self._resolve_second_relay() + self._notify_status_webhook("reboot", user=prior_user) # log to Slack, if enabled slack: Optional["SlackHandler"] = current_app.config.get("SLACK_HANDLER") if not slack: @@ -977,16 +1017,19 @@ async def _handle_oops(self, users: UsersConfig) -> None: """Handle oops button press.""" ustr: str = "" uname: Optional[str] = None + oops_user: Optional[User] = None if self.rfid_value: ustr = " RFID card is present but unknown." if user := users.users_by_fob.get(self.rfid_value): ustr = f" Current user is: {user.full_name}." uname = user.full_name + oops_user = user logging.getLogger("OOPS").warning( "Machine %s was Oopsed.%s", self.machine.display_name, ustr ) # locking handled in update() self.oops(do_locking=False) + self._notify_status_webhook("oops", user=oops_user) # log to Slack, if enabled slack: Optional["SlackHandler"] = current_app.config.get("SLACK_HANDLER") if not slack: @@ -1002,6 +1045,7 @@ async def _handle_oops(self, users: UsersConfig) -> None: async def _handle_rfid_remove(self) -> None: """Handle RFID card removed.""" was_override: bool = self.is_override_login + departed_user: Optional[User] = self.current_user logging.getLogger("AUTH").info( "RFID logout on %s by %s; session duration %d seconds%s", self.machine.display_name, @@ -1044,6 +1088,7 @@ async def _handle_rfid_remove(self) -> None: self.display_text = self.DEFAULT_DISPLAY_TEXT self.status_led_rgb = (0.0, 0.0, 0.0) self.status_led_brightness = 0.0 + self._notify_status_webhook("logout", user=departed_user) # log to Slack, if enabled slack: Optional["SlackHandler"] = current_app.config.get("SLACK_HANDLER") if not slack: @@ -1074,6 +1119,7 @@ async def _handle_rfid_insert(self, users: UsersConfig, rfid_value: str) -> None self.display_text = "Unknown RFID" self.status_led_rgb = (1.0, 0.0, 0.0) self.status_led_brightness = self.STATUS_LED_BRIGHTNESS + self._notify_status_webhook("unknown_fob") if slack: await slack.admin_log( f"RFID login attempt on {self.machine.display_name} by unknown fob" @@ -1094,6 +1140,7 @@ async def _handle_rfid_insert(self, users: UsersConfig, rfid_value: str) -> None self.display_text = f"OVERRIDE BY\n{user.preferred_name}" self.status_led_rgb = (0.0, 1.0, 0.0) self.status_led_brightness = self.STATUS_LED_BRIGHTNESS + self._notify_status_webhook("override_login", user=user) if slack: await slack.log_override_login(self.machine, user.full_name) return @@ -1139,6 +1186,7 @@ async def _handle_rfid_insert(self, users: UsersConfig, rfid_value: str) -> None # the Slack message with the accessory clause; update() will emit # the structured AUTH log later. self._resolve_second_relay(emit_log=False) + self._notify_status_webhook("login", user=user) if slack: msg = ( f"RFID login on {self.machine.display_name} by authorized user " @@ -1168,6 +1216,7 @@ async def _handle_rfid_insert(self, users: UsersConfig, rfid_value: str) -> None self.display_text = "Unauthorized" self.status_led_rgb = (1.0, 0.5, 0.0) # orange self.status_led_brightness = self.STATUS_LED_BRIGHTNESS + self._notify_status_webhook("unauthorized", user=user) if slack: await slack.admin_log( f"rejected RFID login on {self.machine.display_name} by " @@ -1195,10 +1244,12 @@ async def _handle_rfid_tracking_always_enabled( else 0 ), ) + departed_user: Optional[User] = self.current_user self.rfid_value = None self.rfid_present_since = None self.current_user = None # State remains always-on (relay/display/LED not changed) + self._notify_status_webhook("logout", user=departed_user) else: # RFID inserted self.rfid_present_since = time() @@ -1212,12 +1263,14 @@ async def _handle_rfid_tracking_always_enabled( user.full_name, rfid_value, ) + self._notify_status_webhook("login", user=user) else: logging.getLogger("AUTH").warning( "RFID inserted on always-enabled machine %s by unknown fob %s", self.machine.display_name, rfid_value, ) + self._notify_status_webhook("unknown_fob") # State remains always-on (relay/display/LED not changed) def _user_is_second_authorized(self, user: User) -> bool: diff --git a/src/dm_mac/webhook.py b/src/dm_mac/webhook.py new file mode 100644 index 0000000..4d24295 --- /dev/null +++ b/src/dm_mac/webhook.py @@ -0,0 +1,127 @@ +"""Status-change webhook notifier for external consumers (e.g. the ESB).""" + +import logging +import os +from asyncio import CancelledError +from asyncio import create_task +from asyncio import sleep +from time import time +from typing import TYPE_CHECKING +from typing import Any +from typing import Dict +from typing import Optional + +import aiohttp + +if TYPE_CHECKING: # pragma: no cover + from dm_mac.models.machine import Machine + from dm_mac.models.users import User + + +logger: logging.Logger = logging.getLogger(__name__) + + +class WebhookNotifier: + """Fire status-change webhooks to a configured URL. + + Enabled only when ``STATUS_WEBHOOK_URL`` is set (see :meth:`from_env`). + Deliveries are fire-and-forget: :meth:`notify` spawns an + :func:`asyncio.create_task` so callers -- the MCU update handler and the + Slack command handlers -- never block on the HTTP request, exactly as the + Slack notifications do. Each delivery retries with exponential backoff and a + per-attempt timeout; once the retries are exhausted the failure is logged + and dropped (external consumers can reconcile via ``GET /api/machines``). + + The webhook only fires on meaningful status changes (login, logout, + unauthorized/unknown fob, override login, oops, un-oops, lockout, unlock, + reboot) because :meth:`notify` is called only from those code paths, never + from ordinary MCU heartbeat updates. + """ + + #: Total number of POST attempts per event (initial try plus retries). + MAX_ATTEMPTS: int = 4 + #: Base backoff in seconds; before attempt N (1-indexed) we wait + #: ``BACKOFF_BASE_SEC * 2 ** (N - 2)`` (no wait before the first attempt). + BACKOFF_BASE_SEC: float = 1.0 + #: Per-attempt HTTP timeout in seconds. + REQUEST_TIMEOUT_SEC: float = 5.0 + + def __init__(self, url: str): + """Initialize a WebhookNotifier posting to ``url``.""" + self.url: str = url + + @classmethod + def from_env(cls) -> Optional["WebhookNotifier"]: + """Build a notifier from ``STATUS_WEBHOOK_URL``, or None if unset.""" + url: str = os.environ.get("STATUS_WEBHOOK_URL", "").strip() + if not url: + return None + logger.info("Status-change webhook enabled; posting to %s", url) + return cls(url) + + def build_payload( + self, machine: "Machine", event: str, user: Optional["User"] = None + ) -> Dict[str, Any]: + """Build the webhook payload for ``event`` on ``machine``. + + The payload is :py:attr:`Machine.status_dict ` (so ``current_user`` means the same thing here as + in ``GET /api/machines``) plus three fields: + + * ``event`` -- the status-change event name. + * ``timestamp`` -- epoch seconds when the event fired. + * ``user`` -- the user *involved in this event* (the actor), as + ``{"account_id", "full_name"}`` or ``None``. This differs from + ``current_user`` for events like ``logout``/``unauthorized`` where a + user acted but is not (or no longer) logged in. + """ + payload: Dict[str, Any] = machine.status_dict + payload["event"] = event + payload["timestamp"] = time() + payload["user"] = ( + None + if user is None + else {"account_id": user.account_id, "full_name": user.full_name} + ) + return payload + + def notify( + self, machine: "Machine", event: str, user: Optional["User"] = None + ) -> None: + """Build the payload and spawn a fire-and-forget delivery task.""" + payload: Dict[str, Any] = self.build_payload(machine, event, user=user) + create_task(self._deliver(payload)) + + async def _deliver(self, payload: Dict[str, Any]) -> None: + """Deliver one webhook with retries and exponential backoff. + + Returns as soon as an attempt gets a non-error (< 400) response. On + network errors or 4xx/5xx responses it retries up to + :data:`MAX_ATTEMPTS` times, backing off exponentially between attempts, + then logs a single error and gives up. + """ + timeout: aiohttp.ClientTimeout = aiohttp.ClientTimeout( + total=self.REQUEST_TIMEOUT_SEC + ) + last_err: Optional[str] = None + for attempt in range(1, self.MAX_ATTEMPTS + 1): + if attempt > 1: + await sleep(self.BACKOFF_BASE_SEC * (2 ** (attempt - 2))) + try: + async with aiohttp.ClientSession(timeout=timeout) as session: + async with session.post(self.url, json=payload) as resp: + if resp.status < 400: + return + last_err = f"HTTP {resp.status}" + except CancelledError: # pragma: no cover + raise + except Exception as ex: + last_err = repr(ex) + logger.error( + "Failed to deliver status webhook for machine %s event %s after " + "%d attempts: %s", + payload.get("name"), + payload.get("event"), + self.MAX_ATTEMPTS, + last_err, + ) diff --git a/tests/test_webhook.py b/tests/test_webhook.py new file mode 100644 index 0000000..7588f62 --- /dev/null +++ b/tests/test_webhook.py @@ -0,0 +1,199 @@ +"""Tests for dm_mac.webhook.WebhookNotifier.""" + +import asyncio +from typing import Any +from typing import Dict +from typing import List +from typing import Optional +from unittest.mock import AsyncMock +from unittest.mock import Mock +from unittest.mock import patch + +from dm_mac.webhook import WebhookNotifier + +pbm: str = "dm_mac.webhook" + + +def _machine(status_dict: Optional[Dict[str, Any]] = None) -> Mock: + """Build a fake Machine whose status_dict returns a fresh dict each call.""" + if status_dict is None: + status_dict = { + "name": "planer", + "display_name": "Planer", + "status": "idle", + "relay": False, + "oops": False, + "locked_out": False, + "current_user": None, + "last_checkin": 100.0, + "last_update": 200.0, + } + mach = Mock() + # Return a copy each access so build_payload's mutation doesn't leak. + mach.status_dict = dict(status_dict) + return mach + + +class _FakePost: + """Async context manager standing in for ``session.post(...)``.""" + + def __init__(self, status: Optional[int] = None, exc: Optional[Exception] = None): + self.status = status + self._exc = exc + + async def __aenter__(self) -> "_FakePost": + if self._exc is not None: + raise self._exc + return self + + async def __aexit__(self, *a: Any) -> bool: + return False + + +class _FakeSession: + """Async context manager standing in for ``aiohttp.ClientSession(...)``.""" + + #: Records ``(url, json)`` for every post across all instances. + calls: List[Any] = [] + + def __init__(self, outcomes: List[Any]): + self._outcomes = outcomes + self._i = 0 + + async def __aenter__(self) -> "_FakeSession": + return self + + async def __aexit__(self, *a: Any) -> bool: + return False + + def post(self, url: str, json: Dict[str, Any]) -> _FakePost: + _FakeSession.calls.append((url, json)) + outcome = self._outcomes[self._i] + self._i += 1 + if isinstance(outcome, Exception): + return _FakePost(exc=outcome) + return _FakePost(status=outcome) + + +class TestFromEnv: + """Tests for WebhookNotifier.from_env.""" + + def test_unset_returns_none(self) -> None: + """Notifier is disabled (None) when STATUS_WEBHOOK_URL is unset.""" + with patch.dict("os.environ", {}, clear=False): + import os + + os.environ.pop("STATUS_WEBHOOK_URL", None) + assert WebhookNotifier.from_env() is None + + def test_blank_returns_none(self) -> None: + """Whitespace-only URL is treated as unset.""" + with patch.dict("os.environ", {"STATUS_WEBHOOK_URL": " "}): + assert WebhookNotifier.from_env() is None + + def test_set_returns_instance(self) -> None: + """A configured URL yields a notifier pointed at it.""" + with patch.dict("os.environ", {"STATUS_WEBHOOK_URL": "https://esb/hook"}): + notifier = WebhookNotifier.from_env() + assert notifier is not None + assert notifier.url == "https://esb/hook" + + +class TestBuildPayload: + """Tests for WebhookNotifier.build_payload.""" + + def test_payload_no_user(self) -> None: + """Payload is status_dict plus event, timestamp, and null user.""" + notifier = WebhookNotifier("https://esb/hook") + with patch(f"{pbm}.time", return_value=1234.5): + payload = notifier.build_payload(_machine(), "lockout") + assert payload["event"] == "lockout" + assert payload["timestamp"] == 1234.5 + assert payload["user"] is None + assert payload["current_user"] is None + assert payload["name"] == "planer" + assert payload["status"] == "idle" + + def test_payload_with_actor_user(self) -> None: + """The acting user populates ``user`` distinct from ``current_user``.""" + notifier = WebhookNotifier("https://esb/hook") + user = Mock(account_id="42", full_name="Jane Doe") + # logout: nobody is currently logged in, but Jane is the actor. + payload = notifier.build_payload(_machine(), "logout", user=user) + assert payload["event"] == "logout" + assert payload["user"] == {"account_id": "42", "full_name": "Jane Doe"} + assert payload["current_user"] is None + + +class TestNotify: + """Tests for WebhookNotifier.notify (fire-and-forget scheduling).""" + + async def test_notify_spawns_delivery(self) -> None: + """notify builds the payload and schedules _deliver with it.""" + notifier = WebhookNotifier("https://esb/hook") + user = Mock(account_id="1", full_name="Ashley") + with patch.object( + WebhookNotifier, "_deliver", new_callable=AsyncMock + ) as mock_deliver: + notifier.notify(_machine(), "login", user=user) + await asyncio.sleep(0) # let the scheduled task run + mock_deliver.assert_awaited_once() + payload = mock_deliver.await_args.args[0] + assert payload["event"] == "login" + assert payload["user"] == {"account_id": "1", "full_name": "Ashley"} + + +class TestDeliver: + """Tests for WebhookNotifier._deliver (retry/backoff).""" + + def setup_method(self) -> None: + """Reset the shared post-call recorder before each test.""" + _FakeSession.calls = [] + + async def test_success_first_try(self) -> None: + """A 2xx on the first attempt posts once with no backoff.""" + notifier = WebhookNotifier("https://esb/hook") + session = _FakeSession([200]) + sleep_mock = AsyncMock() + with patch(f"{pbm}.aiohttp.ClientSession", return_value=session), patch( + f"{pbm}.sleep", sleep_mock + ): + await notifier._deliver({"name": "planer", "event": "login"}) + assert len(_FakeSession.calls) == 1 + sleep_mock.assert_not_awaited() + + async def test_retries_then_succeeds(self) -> None: + """A network error then a 200 retries once (with one backoff).""" + notifier = WebhookNotifier("https://esb/hook") + session = _FakeSession([OSError("boom"), 200]) + sleep_mock = AsyncMock() + with patch(f"{pbm}.aiohttp.ClientSession", return_value=session), patch( + f"{pbm}.sleep", sleep_mock + ): + await notifier._deliver({"name": "planer", "event": "login"}) + assert len(_FakeSession.calls) == 2 + sleep_mock.assert_awaited_once() + # First backoff uses BACKOFF_BASE_SEC * 2**0 == BACKOFF_BASE_SEC. + assert sleep_mock.await_args.args[0] == notifier.BACKOFF_BASE_SEC + + async def test_http_error_retried(self) -> None: + """A 4xx/5xx status is treated as failure and retried.""" + notifier = WebhookNotifier("https://esb/hook") + session = _FakeSession([500, 503, 200]) + with patch(f"{pbm}.aiohttp.ClientSession", return_value=session), patch( + f"{pbm}.sleep", AsyncMock() + ): + await notifier._deliver({"name": "planer", "event": "oops"}) + assert len(_FakeSession.calls) == 3 + + async def test_all_attempts_fail_logs_error(self) -> None: + """After MAX_ATTEMPTS failures it gives up and logs a single error.""" + notifier = WebhookNotifier("https://esb/hook") + outcomes: List[Any] = [OSError("x")] * notifier.MAX_ATTEMPTS + session = _FakeSession(outcomes) + with patch(f"{pbm}.aiohttp.ClientSession", return_value=session), patch( + f"{pbm}.sleep", AsyncMock() + ), patch(f"{pbm}.logger") as mock_logger: + await notifier._deliver({"name": "planer", "event": "login"}) + assert len(_FakeSession.calls) == notifier.MAX_ATTEMPTS + mock_logger.error.assert_called_once() diff --git a/tests/views/test_webhook_wiring.py b/tests/views/test_webhook_wiring.py new file mode 100644 index 0000000..d9c637b --- /dev/null +++ b/tests/views/test_webhook_wiring.py @@ -0,0 +1,158 @@ +"""Integration tests that the status webhook fires from the update path.""" + +from pathlib import Path +from typing import Any +from typing import Dict +from typing import List +from unittest.mock import AsyncMock +from unittest.mock import Mock + +from quart import Quart +from quart.typing import TestClientProtocol + +from .quart_test_helpers import app_and_client + + +def _heartbeat(machine_name: str, **overrides: Any) -> Dict[str, Any]: + """Return a minimal /api/machine/update payload.""" + body: Dict[str, Any] = { + "machine_name": machine_name, + "oops": False, + "rfid_value": "", + "uptime": 100.0, + "wifi_signal_db": -50, + "wifi_signal_percent": 90, + "internal_temperature_c": 50.0, + } + body.update(overrides) + return body + + +def _events(notify: Mock) -> List[str]: + """Return the ordered list of event names passed to notifier.notify.""" + return [call.args[1] for call in notify.call_args_list] + + +class TestWebhookWiring: + """Verify meaningful updates fire the webhook and heartbeats do not.""" + + def _app_with_notifier(self, tmp_path: Path) -> Any: + app: Quart + client: TestClientProtocol + app, client = app_and_client(tmp_path) + notifier = Mock() + notifier.notify = Mock() + app.config["WEBHOOK_NOTIFIER"] = notifier + return app, client, notifier + + async def test_heartbeat_does_not_fire(self, tmp_path: Path) -> None: + """An empty-RFID heartbeat is not a meaningful change; no webhook.""" + app, client, notifier = self._app_with_notifier(tmp_path) + await client.post("/api/machine/update", json=_heartbeat("metal-mill")) + await client.post("/api/machine/update", json=_heartbeat("metal-mill")) + notifier.notify.assert_not_called() + + async def test_login_then_logout_fire(self, tmp_path: Path) -> None: + """An authorized login and the following logout each fire once.""" + app, client, notifier = self._app_with_notifier(tmp_path) + # Ashley Williams (fob 8114346998) is authorized for Metal Mill. + await client.post( + "/api/machine/update", + json=_heartbeat("metal-mill", rfid_value="8114346998"), + ) + await client.post("/api/machine/update", json=_heartbeat("metal-mill")) + assert _events(notifier.notify) == ["login", "logout"] + # login carries the acting user + login_call = notifier.notify.call_args_list[0] + assert login_call.kwargs["user"].full_name == "Ashley Williams" + # logout carries the departed user + logout_call = notifier.notify.call_args_list[1] + assert logout_call.kwargs["user"].full_name == "Ashley Williams" + + async def test_unauthorized_fires(self, tmp_path: Path) -> None: + """An unauthorized fob on a restrictive machine fires 'unauthorized'.""" + app, client, notifier = self._app_with_notifier(tmp_path) + # Kenneth Hunter (0091703745) lacks Metal Lathe on restrictive-lathe. + await client.post( + "/api/machine/update", + json=_heartbeat("restrictive-lathe", rfid_value="0091703745"), + ) + assert _events(notifier.notify) == ["unauthorized"] + + async def test_unknown_fob_fires(self, tmp_path: Path) -> None: + """An unknown fob fires 'unknown_fob' with no user.""" + app, client, notifier = self._app_with_notifier(tmp_path) + await client.post( + "/api/machine/update", + json=_heartbeat("metal-mill", rfid_value="9999999999"), + ) + assert _events(notifier.notify) == ["unknown_fob"] + assert notifier.notify.call_args_list[0].kwargs["user"] is None + + async def test_oops_button_fires(self, tmp_path: Path) -> None: + """Pressing the oops button fires 'oops'.""" + app, client, notifier = self._app_with_notifier(tmp_path) + await client.post( + "/api/machine/update", json=_heartbeat("metal-mill", oops=True) + ) + assert _events(notifier.notify) == ["oops"] + + async def test_api_oops_and_clear_fire(self, tmp_path: Path) -> None: + """POST/DELETE oops control endpoints fire 'oops'/'unoops'.""" + app, client, notifier = self._app_with_notifier(tmp_path) + await client.post("/api/machine/oops/metal-mill") + await client.delete("/api/machine/oops/metal-mill") + assert _events(notifier.notify) == ["oops", "unoops"] + + async def test_api_lockout_and_unlock_fire(self, tmp_path: Path) -> None: + """POST/DELETE lockout control endpoints fire 'lockout'/'unlock'.""" + app, client, notifier = self._app_with_notifier(tmp_path) + await client.post("/api/machine/locked_out/metal-mill") + await client.delete("/api/machine/locked_out/metal-mill") + assert _events(notifier.notify) == ["lockout", "unlock"] + + async def test_always_enabled_login_logout(self, tmp_path: Path) -> None: + """Always-on machines still fire login/logout for the tracked user.""" + app, client, notifier = self._app_with_notifier(tmp_path) + # known user login on the always-on machine, then removal + await client.post( + "/api/machine/update", + json=_heartbeat("always-on-machine", rfid_value="8114346998"), + ) + await client.post("/api/machine/update", json=_heartbeat("always-on-machine")) + assert _events(notifier.notify) == ["login", "logout"] + + async def test_always_enabled_unknown_fob(self, tmp_path: Path) -> None: + """An unknown fob on an always-on machine fires 'unknown_fob'.""" + app, client, notifier = self._app_with_notifier(tmp_path) + await client.post( + "/api/machine/update", + json=_heartbeat("always-on-machine", rfid_value="9999999999"), + ) + assert _events(notifier.notify) == ["unknown_fob"] + + async def test_slack_path_fires_via_handler_app(self, tmp_path: Path) -> None: + """A Slack-initiated oops (no request context) fires via slack.quart.""" + app, client = app_and_client(tmp_path) + notifier = Mock() + notifier.notify = Mock() + app.config["WEBHOOK_NOTIFIER"] = notifier + # Fake Slack handler holding the app, as the real SlackHandler does. + fake_slack = Mock() + fake_slack.quart = app + fake_slack.log_oops = AsyncMock() + mach = app.config["MACHINES"].machines_by_name["metal-mill"] + # Called outside any request context, exactly like the Slack callback. + await mach.oops(slack=fake_slack) + assert _events(notifier.notify) == ["oops"] + fake_slack.log_oops.assert_awaited_once() + + async def test_disabled_notifier_is_noop(self, tmp_path: Path) -> None: + """With no notifier configured, meaningful updates still succeed.""" + app, client = app_and_client(tmp_path) + assert app.config["WEBHOOK_NOTIFIER"] is None + resp = await client.post( + "/api/machine/update", + json=_heartbeat("metal-mill", rfid_value="8114346998"), + ) + assert resp.status_code == 200 From 9d2ddc23f35568de687742d0c7743c646cd9ca4b Mon Sep 17 00:00:00 2001 From: Jason Antman Date: Sat, 4 Jul 2026 09:09:10 -0400 Subject: [PATCH 4/9] ESB Support - 3: docs, dependency fix, and feature completion Implements Milestone 3 (Acceptance Criteria) and completes the ESB support feature. - Document the new surface: STATUS_WEBHOOK_URL in the configuration env-var table; a "Machine Status API" section (GET /api/machines) and a "Status-Change Webhook" section (payload, events, delivery semantics) in docs/source/http-api.rst; and app-config, API endpoint, env-var, and Status-Change Webhook notes in CLAUDE.md. GET /api/machines is picked up by the generated OpenAPI spec and dm_mac.webhook by sphinx-apidoc. - Pin msgpack ^1.2.1 in pyproject.toml's existing "secure versions of transitive dependencies" block to clear a pre-existing GHSA-6v7p-g79w-8964 advisory (msgpack 1.2.0) that was failing `nox -s safety` on main, unrelated to this feature, so the "all nox sessions passing" acceptance criterion is met. - Move docs/features/esb-support.md to docs/features/completed/. All nox sessions pass (tests, mypy, pre-commit, typeguard, docs, safety); webhook.py at 100% coverage, 98% overall. Co-Authored-By: Claude Opus 4.8 (1M context) --- CLAUDE.md | 24 ++++ docs/features/{ => completed}/esb-support.md | 17 ++- docs/source/configuration.rst | 3 + docs/source/dm_mac.rst | 1 + docs/source/dm_mac.webhook.rst | 8 ++ docs/source/http-api.rst | 84 +++++++++++ poetry.lock | 138 +++++++++---------- pyproject.toml | 1 + 8 files changed, 206 insertions(+), 70 deletions(-) rename docs/features/{ => completed}/esb-support.md (89%) create mode 100644 docs/source/dm_mac.webhook.rst diff --git a/CLAUDE.md b/CLAUDE.md index da5d83d..9ac08a7 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -110,6 +110,7 @@ On every push to `main`, `.github/workflows/release.yml` compares the `pyproject - `MACHINES`: MachinesConfig instance managing all machine configurations - `USERS`: UsersConfig instance managing all user data - `SLACK_HANDLER`: Optional SlackHandler for Slack integration +- `WEBHOOK_NOTIFIER`: Optional WebhookNotifier (see Status-Change Webhook below); `None` unless `STATUS_WEBHOOK_URL` is set - `START_TIME`: Server start timestamp for uptime tracking **Configuration System**: @@ -196,8 +197,30 @@ as `/api/machine/*`): **Admin APIs** (`/api/*`): - `POST /api/reload-users`: Hot-reload users.json without restart +- `GET /api/machines`: List all machines and their current status as JSON + (sorted by name). Read-only; intended for external consumers such as the + Equipment Status Board. Each entry is `Machine.status_dict` (name, + display_name, derived `status`, relay, oops, locked_out, current_user, + last_checkin, last_update). - `GET /metrics`: Prometheus metrics endpoint +### Status-Change Webhook + +When `STATUS_WEBHOOK_URL` is set, `WebhookNotifier` (`src/dm_mac/webhook.py`) +POSTs a JSON webhook to that URL on every *meaningful* machine status change — +never on ordinary MCU heartbeats. Events: `login`, `logout`, `unauthorized`, +`unknown_fob`, `override_login`, `oops`, `unoops`, `lockout`, `unlock`, +`reboot`. The payload is `Machine.status_dict` plus `event`, `timestamp`, and a +distinct `user` field (the event actor, e.g. who logged out — differs from +`current_user`). Delivery is fire-and-forget (an `asyncio` task, like the Slack +notifications) with a per-attempt timeout and bounded exponential-backoff +retry, so the MCU response is never blocked; failures are logged and dropped. +`notify()` is called only from the meaningful status-change code paths in +`models/machine.py`, so heartbeats never fire it. The notifier is resolved via +`current_app` in request-context (MCU/API) paths and via the passed Slack +handler's app (`slack.quart`) in the Slack-command path, which runs without a +request context. + ### Logging Custom `RequestFormatter` adds request context (`remote_addr`, `url`) to all logs when available. The `AUTH` logger is used specifically for authentication/authorization decisions. @@ -218,6 +241,7 @@ Optional for MAC server: - `SLACK_SIGNING_SECRET`: Slack Signing Secret - `SLACK_CONTROL_CHANNEL_ID`: Private admin channel ID - `SLACK_OOPS_CHANNEL_ID`: Public channel for oops/maintenance notices +- `STATUS_WEBHOOK_URL`: If set, URL to POST status-change webhooks to (see Status-Change Webhook above); disabled when unset ## Testing Notes diff --git a/docs/features/esb-support.md b/docs/features/completed/esb-support.md similarity index 89% rename from docs/features/esb-support.md rename to docs/features/completed/esb-support.md index 889776a..5872ec7 100644 --- a/docs/features/esb-support.md +++ b/docs/features/completed/esb-support.md @@ -141,7 +141,22 @@ tests cover the notifier (payload/retry/backoff/disabled) and the wiring * **2.4** Unit tests: payload shape per event, disabled-when-unset, retry/backoff on failure (mock ``aiohttp``), and non-firing on ordinary heartbeat updates. -### Milestone 3 — Acceptance Criteria +### Milestone 3 — Acceptance Criteria — ✅ COMPLETE + +Updated documentation (`docs/source/configuration.rst` env-var table; +`docs/source/http-api.rst` with the Machine Status API and Status-Change +Webhook sections; `CLAUDE.md` app-config, API endpoints, env vars, and a new +Status-Change Webhook subsection). `README.rst` is a pointer to the full docs +and needed no change. `GET /api/machines` is picked up automatically by the +generated OpenAPI spec, and `dm_mac.webhook` by `sphinx-apidoc`. New code has +unit-test coverage (`webhook.py` at 100%). All `nox` sessions pass: `tests`, +`mypy`, `pre-commit`, `typeguard`, `docs`, and `safety`. + +Note: `nox -s safety` was failing on `main` due to a pre-existing transitive +`msgpack==1.2.0` advisory (GHSA-6v7p-g79w-8964), unrelated to this feature. To +satisfy the "all nox sessions passing" criterion, `msgpack ^1.2.1` was pinned +in `pyproject.toml`'s existing "secure versions of transitive dependencies" +block (same mechanism already used for urllib3/werkzeug/marshmallow). * **3.1** Update documentation: ``README.md`` (env var + endpoints if listed), ``docs/source/configuration.rst`` (add ``STATUS_WEBHOOK_URL`` to the env-var diff --git a/docs/source/configuration.rst b/docs/source/configuration.rst index 6f79c93..17f4a08 100644 --- a/docs/source/configuration.rst +++ b/docs/source/configuration.rst @@ -99,6 +99,9 @@ Environment Variables * - ``SLACK_OOPS_CHANNEL_ID`` - no - If using the Slack integration, the Channel ID of of the public channel where Oops and maintenance notices will be posted, and where machine status can be checked. + * - ``STATUS_WEBHOOK_URL`` + - no + - If set, the URL to POST a JSON status-change webhook to on every meaningful machine event (login, logout, unauthorized or unknown fob, override login, oops, un-oops, lockout, unlock, reboot). Disabled when unset. See :ref:`http-api.status-webhook`. .. _configuration.machine-state-dir: diff --git a/docs/source/dm_mac.rst b/docs/source/dm_mac.rst index 953ccdf..92f9df8 100644 --- a/docs/source/dm_mac.rst +++ b/docs/source/dm_mac.rst @@ -27,3 +27,4 @@ Submodules dm_mac.neongetter dm_mac.slack_handler dm_mac.utils + dm_mac.webhook diff --git a/docs/source/dm_mac.webhook.rst b/docs/source/dm_mac.webhook.rst new file mode 100644 index 0000000..ac58c5e --- /dev/null +++ b/docs/source/dm_mac.webhook.rst @@ -0,0 +1,8 @@ +dm\_mac.webhook module +====================== + +.. automodule:: dm_mac.webhook + :members: + :private-members: + :show-inheritance: + :undoc-members: diff --git a/docs/source/http-api.rst b/docs/source/http-api.rst index 9403cec..f7f8fbb 100644 --- a/docs/source/http-api.rst +++ b/docs/source/http-api.rst @@ -82,6 +82,90 @@ The ``display`` field is byte-identical between pre-feature and post-feature servers for any given (machine, operator, machine state) tuple. ``second_relay`` configuration never causes LCD changes. +Machine Status API +------------------ + +``GET /api/machines`` + +Returns the current status of every configured machine as JSON, sorted by +machine name. This read-only endpoint is intended for external consumers (such +as the `Equipment Status Board +`_) to poll or +reconcile machine state. It is included in the OpenAPI spec above. Each machine +entry has the shape: + +:: + + { + "name": "planer", + "display_name": "Planer", + "status": "in_use", + "relay": true, + "oops": false, + "locked_out": false, + "current_user": {"account_id": "123", "full_name": "Jane Doe"}, + "last_checkin": 1720000000.0, + "last_update": 1720000000.0 + } + +``status`` is a derived summary — one of ``locked_out``, ``oops``, ``in_use``, +``idle``, or ``unknown`` (never checked in). ``current_user`` is ``null`` when +no user is logged in. ``last_checkin`` / ``last_update`` are epoch seconds, or +``null`` if the machine has never checked in. + +.. _http-api.status-webhook: + +Status-Change Webhook +--------------------- + +When the ``STATUS_WEBHOOK_URL`` environment variable is set (see +:ref:`configuration.env-vars`), the server POSTs a JSON webhook to that URL on +every *meaningful* machine status change — never on ordinary MCU heartbeats. +This lets an external consumer (such as the Equipment Status Board) maintain +its own activity history and live status without polling. + +The webhook fires for these ``event`` values: ``login``, ``logout``, +``unauthorized`` (known user lacking authorization), ``unknown_fob``, +``override_login``, ``oops``, ``unoops``, ``lockout``, ``unlock``, and +``reboot`` (MCU reboot detected). + +The request body is the same per-machine object as ``GET /api/machines`` (so +``current_user`` means the same thing) plus three fields: + +* ``event`` — the status-change event name (see above). +* ``timestamp`` — epoch seconds when the event fired. +* ``user`` — the user *involved in this event* (the actor), as + ``{"account_id", "full_name"}`` or ``null``. This differs from + ``current_user`` for events like ``logout`` and ``unauthorized`` where a user + acted but is not (or is no longer) logged in. + +:: + + POST + Content-Type: application/json + + { + "name": "planer", + "display_name": "Planer", + "status": "idle", + "relay": false, + "oops": false, + "locked_out": false, + "current_user": null, + "last_checkin": 1720000000.0, + "last_update": 1720000000.5, + "event": "logout", + "timestamp": 1720000000.5, + "user": {"account_id": "123", "full_name": "Jane Doe"} + } + +Delivery is fire-and-forget so it never blocks the MCU response: each webhook +is sent on a background task that retries with exponential backoff and a +per-attempt timeout, giving up (and logging an error) after a few attempts. No +authentication header is sent. Consumers should treat delivery as best-effort +and reconcile via ``GET /api/machines`` when needed. See +:py:mod:`dm_mac.webhook` for details. + Prometheus Metrics ------------------ diff --git a/poetry.lock b/poetry.lock index ab95c8a..71188d5 100644 --- a/poetry.lock +++ b/poetry.lock @@ -2273,78 +2273,78 @@ files = [ [[package]] name = "msgpack" -version = "1.2.0" +version = "1.2.1" description = "MessagePack serializer" optional = false python-versions = ">=3.10" -groups = ["dev"] +groups = ["main", "dev"] files = [ - {file = "msgpack-1.2.0-cp310-cp310-macosx_10_9_x86_64.whl", hash = "sha256:ed8c9495a0f12d17a2b4b69e23f895b88f26aabe40911c86594d3fbddecfff08"}, - {file = "msgpack-1.2.0-cp310-cp310-macosx_11_0_arm64.whl", hash = "sha256:d7384859c90b45a28a4b31aa50b49cca84504c9f27df459cea6e072627650dcb"}, - {file = "msgpack-1.2.0-cp310-cp310-manylinux2014_aarch64.manylinux_2_17_aarch64.manylinux_2_28_aarch64.whl", hash = "sha256:63b35e8e65f04ff7ad5c9c70885da587c74f51e4b4eb3db624eac6d250e8cf59"}, - {file = "msgpack-1.2.0-cp310-cp310-manylinux2014_x86_64.manylinux_2_17_x86_64.manylinux_2_28_x86_64.whl", hash = "sha256:9004c5a02acd3eca4e15e1ae7b461c32e3711105a28b1ad78be2f6facff4c523"}, - {file = "msgpack-1.2.0-cp310-cp310-manylinux_2_31_riscv64.manylinux_2_39_riscv64.whl", hash = "sha256:7e2032dacb0a973fcbf7bd088415a369dae31c5af40e199d234806be22e86765"}, - {file = "msgpack-1.2.0-cp310-cp310-musllinux_1_2_aarch64.whl", hash = "sha256:c1feb100651fbe4b39826207cb20af065dfbfbfa43b1bafd7eaa2252abf7acfd"}, - {file = "msgpack-1.2.0-cp310-cp310-musllinux_1_2_riscv64.whl", hash = "sha256:82487709d4c597d252311a65370220675fb1cc859e7da9269a3060c03ac02cf6"}, - {file = "msgpack-1.2.0-cp310-cp310-musllinux_1_2_x86_64.whl", hash = "sha256:0268c67a74f5f913f545a0fdbbfaa3f6ebcf23b4c3209bb99704a2ea87e13f90"}, - {file = "msgpack-1.2.0-cp310-cp310-win32.whl", hash = "sha256:7df87173b0e13ddd134919731f13525dbbf75204145597decf1cb86887ebb492"}, - {file = "msgpack-1.2.0-cp310-cp310-win_amd64.whl", hash = "sha256:6371edb47788fbfd8a22016f9a97b5616dd9849bc50abcbb8e82d38f71efa096"}, - {file = "msgpack-1.2.0-cp311-cp311-macosx_10_9_x86_64.whl", hash = "sha256:ec35cd3f127f50806aa10c3f74bf27b749f13ddf1d2217964ada8f38042d1653"}, - {file = "msgpack-1.2.0-cp311-cp311-macosx_11_0_arm64.whl", hash = "sha256:317eb298297121bfad9173d748124a04a36af27b6ac39c2bbc1db1ce57608dcf"}, - {file = "msgpack-1.2.0-cp311-cp311-manylinux2014_aarch64.manylinux_2_17_aarch64.manylinux_2_28_aarch64.whl", hash = "sha256:50fe6434de89073273026dd032a62e8b63f8857a261d7a2df5b07c9e72f3a8f7"}, - {file = "msgpack-1.2.0-cp311-cp311-manylinux2014_x86_64.manylinux_2_17_x86_64.manylinux_2_28_x86_64.whl", hash = "sha256:106c6d333ff3d4eda075b7d4b9695d1752c5bcc635e40d0dbaf4e276c9ed80e1"}, - {file = "msgpack-1.2.0-cp311-cp311-manylinux_2_31_riscv64.manylinux_2_39_riscv64.whl", hash = "sha256:67055a611e871cb1bd0acb732f2e9f64ca8155ca0bba1d0a5bb362e7209e5541"}, - {file = "msgpack-1.2.0-cp311-cp311-musllinux_1_2_aarch64.whl", hash = "sha256:ceec7f8e633d5a4b4a32b0416bef90ee3cd1017ea36247f705e523072e576119"}, - {file = "msgpack-1.2.0-cp311-cp311-musllinux_1_2_riscv64.whl", hash = "sha256:7ec5851160a3c2c0f77d68ddec620318cd8e7d88d94f9c058190e8ce0dfa1d31"}, - {file = "msgpack-1.2.0-cp311-cp311-musllinux_1_2_x86_64.whl", hash = "sha256:dd7140f7b09dbe1984a0dff3189375d840247e3e4cf4ac45c5a499b3b599c8d2"}, - {file = "msgpack-1.2.0-cp311-cp311-win32.whl", hash = "sha256:cbfd54018d386da0951c7a2be13de0f58559d251313e613b2155e52ed1cbd8f1"}, - {file = "msgpack-1.2.0-cp311-cp311-win_amd64.whl", hash = "sha256:653373c4614c31463ba486a67776e4bb396af289921bd5353e209534b71467fa"}, - {file = "msgpack-1.2.0-cp311-cp311-win_arm64.whl", hash = "sha256:7a260aea1e5e7d6c7f1d9284c7360d29021627b61dc4dd7df144b81210810537"}, - {file = "msgpack-1.2.0-cp312-cp312-macosx_10_13_x86_64.whl", hash = "sha256:e2d6047ccd11a12c96a69f2bfe026471abef67334c3d0494a93e5310e45140a2"}, - {file = "msgpack-1.2.0-cp312-cp312-macosx_11_0_arm64.whl", hash = "sha256:0347e3ac0dfee99086d3b68fe959da3f5f657c0019ddbaeaaa259a85f8603422"}, - {file = "msgpack-1.2.0-cp312-cp312-manylinux2014_aarch64.manylinux_2_17_aarch64.manylinux_2_28_aarch64.whl", hash = "sha256:25552ff1f2ff3dc8333e27eabb94f702da5929ed0e07969688194a3e9f12e151"}, - {file = "msgpack-1.2.0-cp312-cp312-manylinux2014_x86_64.manylinux_2_17_x86_64.manylinux_2_28_x86_64.whl", hash = "sha256:a0d94420d9d52c56568159a69200af7e45eadb29615fa9d09fada140de1c38c7"}, - {file = "msgpack-1.2.0-cp312-cp312-manylinux_2_31_riscv64.manylinux_2_39_riscv64.whl", hash = "sha256:d16e1f2db4a9eebc07b7cc91898d71e710f2eed8358711a605fee802caff8923"}, - {file = "msgpack-1.2.0-cp312-cp312-musllinux_1_2_aarch64.whl", hash = "sha256:e9cb2e700e85f1e27bbb5c9de6cc1c9a4bc5ac64d5404bdcbcb37a0dc7a947a3"}, - {file = "msgpack-1.2.0-cp312-cp312-musllinux_1_2_riscv64.whl", hash = "sha256:717d0b166dd176a5f786aeafff081f6439680acf5af193eb63e6266c12b04d3d"}, - {file = "msgpack-1.2.0-cp312-cp312-musllinux_1_2_x86_64.whl", hash = "sha256:e87c7a21654d18111eb1a89bd5c42baba42e61887365d9e89585e112b4203f9e"}, - {file = "msgpack-1.2.0-cp312-cp312-win32.whl", hash = "sha256:967e0c891f5f23ab65762f2e5dc95922759c79f1ef99ef4c7e1fdd863e0d0af9"}, - {file = "msgpack-1.2.0-cp312-cp312-win_amd64.whl", hash = "sha256:6c23e33cee28dcffa112ae205661da4636fd7b06bd9ad1559a890623b92d060b"}, - {file = "msgpack-1.2.0-cp312-cp312-win_arm64.whl", hash = "sha256:6eeb771571f63f68045433b1a35c0256b946f31ed62f006997e40b8ad8b735af"}, - {file = "msgpack-1.2.0-cp313-cp313-macosx_10_13_x86_64.whl", hash = "sha256:3a1d30df1f302f2b7a7404afbac2ab76d510036c34cf34dffb01f704a7288e45"}, - {file = "msgpack-1.2.0-cp313-cp313-macosx_11_0_arm64.whl", hash = "sha256:581e317112260d8ca488d490cad9290a5682276f309c41c7de237a85ed8799c8"}, - {file = "msgpack-1.2.0-cp313-cp313-manylinux2014_aarch64.manylinux_2_17_aarch64.manylinux_2_28_aarch64.whl", hash = "sha256:c6827d12eacc16873eba62408a1b7bbe8ecfb4a8f7ed78a631ae9bae6ad43cf2"}, - {file = "msgpack-1.2.0-cp313-cp313-manylinux2014_x86_64.manylinux_2_17_x86_64.manylinux_2_28_x86_64.whl", hash = "sha256:a186027e4279efa4c8bf06ce30605498d7d0d3af0fba0b9799dce85a3fd4a93c"}, - {file = "msgpack-1.2.0-cp313-cp313-manylinux_2_31_riscv64.manylinux_2_39_riscv64.whl", hash = "sha256:a96142c14a11cf1a509e8b9aaf72858a3b742b7613e095ce646913e88ce7bd99"}, - {file = "msgpack-1.2.0-cp313-cp313-musllinux_1_2_aarch64.whl", hash = "sha256:50c220579b68a6085b95408b2eaa486b259520f55d8e363ddc9b5d7ba5a6ac6d"}, - {file = "msgpack-1.2.0-cp313-cp313-musllinux_1_2_riscv64.whl", hash = "sha256:4dcb9d12ab100ecacdfaaf37a3d72fe8392eacc7054afc1916b12d1b747c8446"}, - {file = "msgpack-1.2.0-cp313-cp313-musllinux_1_2_x86_64.whl", hash = "sha256:a804727188ab0ebb237fadb303b743f04925a69d8c3247292d1e33e679767c15"}, - {file = "msgpack-1.2.0-cp313-cp313-win32.whl", hash = "sha256:1a1ac6ae1fe23298f79380e7b144c8a454e5d05616b0096584f353ba2d750114"}, - {file = "msgpack-1.2.0-cp313-cp313-win_amd64.whl", hash = "sha256:1c3c80949d79578f9dc85fd9fb91edfe6694e8a729cd5744634d59d8455fdde3"}, - {file = "msgpack-1.2.0-cp313-cp313-win_arm64.whl", hash = "sha256:fcf8f76fa587c2395fd0057c7232dbf071241f9ad280b235adb7ab585289989e"}, - {file = "msgpack-1.2.0-cp314-cp314-macosx_10_15_x86_64.whl", hash = "sha256:f854fa1a8b55d75d82ef9a905d9cdbeffdf7897c088f6020bd221867da5e56a5"}, - {file = "msgpack-1.2.0-cp314-cp314-macosx_11_0_arm64.whl", hash = "sha256:e90df581f80f53b372d5d9d9349078d729851a3a0d0bd74f53ccb598d01e45b8"}, - {file = "msgpack-1.2.0-cp314-cp314-manylinux2014_aarch64.manylinux_2_17_aarch64.manylinux_2_28_aarch64.whl", hash = "sha256:b276ed50d8ac75d1f134a433ae79af8557d0fa25ee5b4737da533dfc2ce382e8"}, - {file = "msgpack-1.2.0-cp314-cp314-manylinux2014_x86_64.manylinux_2_17_x86_64.manylinux_2_28_x86_64.whl", hash = "sha256:544d972459c92aa32e63b800d07c2d9cf2734a3be29cee3a0b478a622850e9f5"}, - {file = "msgpack-1.2.0-cp314-cp314-manylinux_2_31_riscv64.manylinux_2_39_riscv64.whl", hash = "sha256:a070147cc2cf6b8a891734e0f5c8fe8f70ed8739ab30ba140b058005a6e86af4"}, - {file = "msgpack-1.2.0-cp314-cp314-musllinux_1_2_aarch64.whl", hash = "sha256:7685e23b0f51745a751629c31713fbefdef8896b31b2bb38299dfa4ae6c0740c"}, - {file = "msgpack-1.2.0-cp314-cp314-musllinux_1_2_riscv64.whl", hash = "sha256:b9204daeee8d91a7ae5acf2d2a8e3983be9a3025f38aa21bfaefbd7eea84a7dc"}, - {file = "msgpack-1.2.0-cp314-cp314-musllinux_1_2_x86_64.whl", hash = "sha256:bfc057248609742ebbabf6bcd27fea4fd99c4980584e613c168c9b002318298f"}, - {file = "msgpack-1.2.0-cp314-cp314-win32.whl", hash = "sha256:a3faa7edf2388337ae849239878e92f0298b4dab4488e4f1834062f9d0c410c9"}, - {file = "msgpack-1.2.0-cp314-cp314-win_amd64.whl", hash = "sha256:1a3effc392a57744e4681e55d05f97d5ee7b598747d718340a9b4b8a970c40e1"}, - {file = "msgpack-1.2.0-cp314-cp314-win_arm64.whl", hash = "sha256:56a318f7df6bec7b40928d6b0519961f20a510d8baabf6baa393a70444588f0a"}, - {file = "msgpack-1.2.0-cp314-cp314t-macosx_10_15_x86_64.whl", hash = "sha256:afa4a65ab2097795e771a74a3a81ea49534aaeba874eaf426a3332268e045ae6"}, - {file = "msgpack-1.2.0-cp314-cp314t-macosx_11_0_arm64.whl", hash = "sha256:409550770632bb28daa70a11d0ed5763f7db38f40b06f7db9f11dd2794d01102"}, - {file = "msgpack-1.2.0-cp314-cp314t-manylinux2014_aarch64.manylinux_2_17_aarch64.manylinux_2_28_aarch64.whl", hash = "sha256:bf47e3cd11ce044965a9736a322afdd390b31ed602d1c1b10211d1a841f1d587"}, - {file = "msgpack-1.2.0-cp314-cp314t-manylinux2014_x86_64.manylinux_2_17_x86_64.manylinux_2_28_x86_64.whl", hash = "sha256:204bc9f5d6e59c1718c0a4a84fc8ff71b5b4562faac257c1a68bca611ecf9b72"}, - {file = "msgpack-1.2.0-cp314-cp314t-manylinux_2_31_riscv64.manylinux_2_39_riscv64.whl", hash = "sha256:610154307b27267266368bc1d1c7bb8aeb71da7be9356d403cb2442d9e6399f5"}, - {file = "msgpack-1.2.0-cp314-cp314t-musllinux_1_2_aarch64.whl", hash = "sha256:6799f157bb63e79f11e2e590cfdb28423fc18dd60c270c3914b5b4586ae36f7e"}, - {file = "msgpack-1.2.0-cp314-cp314t-musllinux_1_2_riscv64.whl", hash = "sha256:72bd844902cf0a5ac3af2ef742f253cd0b1e5bcd184f49b4fb9a6a1f7bf305e8"}, - {file = "msgpack-1.2.0-cp314-cp314t-musllinux_1_2_x86_64.whl", hash = "sha256:3c0bd450f78d0d81722c80da6cdbf674a856967870a9db2f6c4debc4d8b3c67c"}, - {file = "msgpack-1.2.0-cp314-cp314t-win32.whl", hash = "sha256:378caf74c4c718dfc17590ce68a6d710ed398ff6fcf08237de23b77755730b55"}, - {file = "msgpack-1.2.0-cp314-cp314t-win_amd64.whl", hash = "sha256:553b42598165c4dd3235994fd6e4b0dfb1ce5f3fd33d94ba9609442643015f38"}, - {file = "msgpack-1.2.0-cp314-cp314t-win_arm64.whl", hash = "sha256:2825bb1da548d214ab8a810906b7dd69a10f3838b615a2cc46e5172d3cb44f6e"}, - {file = "msgpack-1.2.0.tar.gz", hash = "sha256:8e17af38197bf58e7e819041678f6178f4491493f5b8c8580414f40f7c2c3c41"}, + {file = "msgpack-1.2.1-cp310-cp310-macosx_10_9_x86_64.whl", hash = "sha256:8c7b398c56ff125feae96c2737abfec5595f1fa0aa186df60c56040b8accb95c"}, + {file = "msgpack-1.2.1-cp310-cp310-macosx_11_0_arm64.whl", hash = "sha256:1548006a91aa93c5da81f3bdcebc1a0d10cea2d25969754fbe848da622b2b895"}, + {file = "msgpack-1.2.1-cp310-cp310-manylinux2014_aarch64.manylinux_2_17_aarch64.manylinux_2_28_aarch64.whl", hash = "sha256:1dabedcd0f23559f3596428c6589c1cd8c6eaed3a0d720795b07b0225d769203"}, + {file = "msgpack-1.2.1-cp310-cp310-manylinux2014_x86_64.manylinux_2_17_x86_64.manylinux_2_28_x86_64.whl", hash = "sha256:83efa1c898e0fc5380fc0cabbf75164c52e3b5cbb45973710d75821928380c73"}, + {file = "msgpack-1.2.1-cp310-cp310-manylinux_2_31_riscv64.manylinux_2_39_riscv64.whl", hash = "sha256:01e2dd6c9b19d333a00282330cc8a73d38d8dabc306dc5b42cd668c3ac82e833"}, + {file = "msgpack-1.2.1-cp310-cp310-musllinux_1_2_aarch64.whl", hash = "sha256:350cb813d0af6e65d2f7ef0d729f7ff5be5a8bce03665892f43e5883d4ecc1b8"}, + {file = "msgpack-1.2.1-cp310-cp310-musllinux_1_2_riscv64.whl", hash = "sha256:ee1d9ed27d0497b848923746cf762ed2e7db24f4be7eec8e5cbe8c766aa707b7"}, + {file = "msgpack-1.2.1-cp310-cp310-musllinux_1_2_x86_64.whl", hash = "sha256:633727297ed063441fd1cda2288865487f33ad14eeb8831afb5f0c396a62cfce"}, + {file = "msgpack-1.2.1-cp310-cp310-win32.whl", hash = "sha256:298872ecf9e61950f1c6af4ca969b859ee91783bb920ef6e6172697d0c8aad74"}, + {file = "msgpack-1.2.1-cp310-cp310-win_amd64.whl", hash = "sha256:2ff164c1b0bcb740b073b99e945234d0212852fa378e44a208c425379140dbeb"}, + {file = "msgpack-1.2.1-cp311-cp311-macosx_10_9_x86_64.whl", hash = "sha256:29a3f6e9667868429d8240dfd063ea5ffdc1321c13d783aa23827a38de0dcb22"}, + {file = "msgpack-1.2.1-cp311-cp311-macosx_11_0_arm64.whl", hash = "sha256:aded5bdf32609dc7987a49bbbd15a8ef096193f96dd8bbeb791de729e650acf5"}, + {file = "msgpack-1.2.1-cp311-cp311-manylinux2014_aarch64.manylinux_2_17_aarch64.manylinux_2_28_aarch64.whl", hash = "sha256:146ee4e9ce80b365c6d4c47073da9da7bcec473e58194ceee5dd7620ace77e06"}, + {file = "msgpack-1.2.1-cp311-cp311-manylinux2014_x86_64.manylinux_2_17_x86_64.manylinux_2_28_x86_64.whl", hash = "sha256:a28d076ca7c82b9c8728ad90b7147489449557038bed50e4241eb832395169b4"}, + {file = "msgpack-1.2.1-cp311-cp311-manylinux_2_31_riscv64.manylinux_2_39_riscv64.whl", hash = "sha256:7d31c0ac0c640f877804c67cb2bc9f4e23dc2db97e96c2e67fa27d38283b41f8"}, + {file = "msgpack-1.2.1-cp311-cp311-musllinux_1_2_aarch64.whl", hash = "sha256:8ff92d7feeaf5bc26c51495b69e2f99ed97ab79346fb6555f44be7dd2ac6503b"}, + {file = "msgpack-1.2.1-cp311-cp311-musllinux_1_2_riscv64.whl", hash = "sha256:779197a6513bab3c3632265e3d0f7cb3227e62510841a6f34f1eaa37efbb345e"}, + {file = "msgpack-1.2.1-cp311-cp311-musllinux_1_2_x86_64.whl", hash = "sha256:67f6dd22fa72a93752643f07889796d62739a13415ee630169a8ce764f86cf9f"}, + {file = "msgpack-1.2.1-cp311-cp311-win32.whl", hash = "sha256:91054a783328e0ea7954b8771095705c8d2243b814743fbaadf14552c9c52c5d"}, + {file = "msgpack-1.2.1-cp311-cp311-win_amd64.whl", hash = "sha256:2eda0b7ebb1283a98d3e4492ac933c8af6aff59fd3df1c3ed024f536af4b1dc8"}, + {file = "msgpack-1.2.1-cp311-cp311-win_arm64.whl", hash = "sha256:6ee967f7c7e1df2890c671ff2ee51a28ded0efc95da3e507176dee881ce36c66"}, + {file = "msgpack-1.2.1-cp312-cp312-macosx_10_13_x86_64.whl", hash = "sha256:2ef59c659f289eddf8aa6623823f19fa2f40a4029266889eac7a2505dd210c35"}, + {file = "msgpack-1.2.1-cp312-cp312-macosx_11_0_arm64.whl", hash = "sha256:d3567748a5107cb40cdf66a275430c2f87c07777698f4bfd25c35f44d533258c"}, + {file = "msgpack-1.2.1-cp312-cp312-manylinux2014_aarch64.manylinux_2_17_aarch64.manylinux_2_28_aarch64.whl", hash = "sha256:60926b75d00c8e816ef98f3034f484a8bc64242d66839cef4cf7e503142316a0"}, + {file = "msgpack-1.2.1-cp312-cp312-manylinux2014_x86_64.manylinux_2_17_x86_64.manylinux_2_28_x86_64.whl", hash = "sha256:020e881a764b20d8d7ca1a54fc01b8175519d108e3c3f194fddc200bda95951a"}, + {file = "msgpack-1.2.1-cp312-cp312-manylinux_2_31_riscv64.manylinux_2_39_riscv64.whl", hash = "sha256:4202c74688ca06591f78cb18988228bd4cca2cc75d57b60008372892d2f1e6e6"}, + {file = "msgpack-1.2.1-cp312-cp312-musllinux_1_2_aarch64.whl", hash = "sha256:8b267ce94efb76fbd1b3373511420074ee3187f0f7811bf394531de13294735a"}, + {file = "msgpack-1.2.1-cp312-cp312-musllinux_1_2_riscv64.whl", hash = "sha256:e4f1d0f8f98ade9634e01fb704a408f9336c0a8f1117b369f5db83dc7551d8b1"}, + {file = "msgpack-1.2.1-cp312-cp312-musllinux_1_2_x86_64.whl", hash = "sha256:f02cf17a6ca1abe29b5f980644f7551f94d71f2011509b26d8625ce038f0df64"}, + {file = "msgpack-1.2.1-cp312-cp312-win32.whl", hash = "sha256:0c0d9802354507bcba62af19c17918e3eb437cc25e6f50657d511b5856a77aac"}, + {file = "msgpack-1.2.1-cp312-cp312-win_amd64.whl", hash = "sha256:5c24aa15d5963051e1a5c62b12c50cd705992502b5ec1f3bece6046f33c9fc24"}, + {file = "msgpack-1.2.1-cp312-cp312-win_arm64.whl", hash = "sha256:4227224aaec8f7fbcbfbd4272319347b2bb4030366502600f8c45588c5187b07"}, + {file = "msgpack-1.2.1-cp313-cp313-macosx_10_13_x86_64.whl", hash = "sha256:0a70e3cf2804a300d921bb0940426e35f4e489a23adfb77a808892241db0a064"}, + {file = "msgpack-1.2.1-cp313-cp313-macosx_11_0_arm64.whl", hash = "sha256:491cc39455ca765fad51fb451bf2915eb2cf41192ab5801ce8d67c1d614fe056"}, + {file = "msgpack-1.2.1-cp313-cp313-manylinux2014_aarch64.manylinux_2_17_aarch64.manylinux_2_28_aarch64.whl", hash = "sha256:f310233ef7fb9c14e201c93639fe5f5260b005f56f0b29048e999c30935596cc"}, + {file = "msgpack-1.2.1-cp313-cp313-manylinux2014_x86_64.manylinux_2_17_x86_64.manylinux_2_28_x86_64.whl", hash = "sha256:787c9bebb5833e8f6fc8abca3c0597683d8d87f56a8842b6b89c75a5f3176e2d"}, + {file = "msgpack-1.2.1-cp313-cp313-manylinux_2_31_riscv64.manylinux_2_39_riscv64.whl", hash = "sha256:dc871b997a9370d855b7394465f2f350e847a5b806dd38dcc9c989e7d87da155"}, + {file = "msgpack-1.2.1-cp313-cp313-musllinux_1_2_aarch64.whl", hash = "sha256:85f57e960d877f2977f6430896191b04a21f8901b3b4baf2e4604329f4db5402"}, + {file = "msgpack-1.2.1-cp313-cp313-musllinux_1_2_riscv64.whl", hash = "sha256:1233ee2dd0cefba127583de50ea654677277047d238303521db35def3d7b2e7c"}, + {file = "msgpack-1.2.1-cp313-cp313-musllinux_1_2_x86_64.whl", hash = "sha256:e3dc2feb0876209d9c38aa56cb1de169bd6c4348f1aa48271f241226590993e6"}, + {file = "msgpack-1.2.1-cp313-cp313-win32.whl", hash = "sha256:6d09badf350af2be9d189184e04e64cf54ad93569ab3d96fca58bd3e84aad707"}, + {file = "msgpack-1.2.1-cp313-cp313-win_amd64.whl", hash = "sha256:33f14fba63278b714efe6ad07e50ea5f03d91537aa6a1c5f1ceca4cf44013ca9"}, + {file = "msgpack-1.2.1-cp313-cp313-win_arm64.whl", hash = "sha256:afc5febcd4c99effbc02b528e49d6fd0760b2b7d48c05239e345a5fa6e743d9a"}, + {file = "msgpack-1.2.1-cp314-cp314-macosx_10_15_x86_64.whl", hash = "sha256:05f340e47e7e47d2da8db9b53e1bb1d294369e9ef45a747441309f6650b8351d"}, + {file = "msgpack-1.2.1-cp314-cp314-macosx_11_0_arm64.whl", hash = "sha256:810b916696c86ef0deb3b74588480224df4c1b071136c34183e4a2a4284d7ac7"}, + {file = "msgpack-1.2.1-cp314-cp314-manylinux2014_aarch64.manylinux_2_17_aarch64.manylinux_2_28_aarch64.whl", hash = "sha256:ca0dacff965c47afdc3749a8469d7302a8f801d6a28758d55120d75e66ce6889"}, + {file = "msgpack-1.2.1-cp314-cp314-manylinux2014_x86_64.manylinux_2_17_x86_64.manylinux_2_28_x86_64.whl", hash = "sha256:0e2bf9280bceb5efca998435904b5d3e9fdbcc11d90dc9df30aec7973252b720"}, + {file = "msgpack-1.2.1-cp314-cp314-manylinux_2_31_riscv64.manylinux_2_39_riscv64.whl", hash = "sha256:aa6c4be5d1c02a42b066ca6ddb71adf36432868fdcdb6ee87e634e86e0674190"}, + {file = "msgpack-1.2.1-cp314-cp314-musllinux_1_2_aarch64.whl", hash = "sha256:ec0e675d59150a6269ddc9139087c722292664a37d071a849c05c473350f1f2d"}, + {file = "msgpack-1.2.1-cp314-cp314-musllinux_1_2_riscv64.whl", hash = "sha256:dd3bfe82d53edfe4b7fc9a7ec9761e23a7a5b1dac22264505af428253c29ed24"}, + {file = "msgpack-1.2.1-cp314-cp314-musllinux_1_2_x86_64.whl", hash = "sha256:5ad5467fc3f68b5468e06c5f788d712e9f8ffc8b0cd1bcb160c105c1ee92dae7"}, + {file = "msgpack-1.2.1-cp314-cp314-win32.whl", hash = "sha256:98b58bdb89c46190e4609bb36abe17c6d4105ad13f9c5f8f6f64d320f8ced3fb"}, + {file = "msgpack-1.2.1-cp314-cp314-win_amd64.whl", hash = "sha256:74847557e28ce71bd3c438a447ca90e4b507e997ddbdef8a12a7b283b86c156b"}, + {file = "msgpack-1.2.1-cp314-cp314-win_arm64.whl", hash = "sha256:b50b727bd652bdc37d950336c848ef20ec54a4cafc38dce19b1cd86ad625d0f7"}, + {file = "msgpack-1.2.1-cp314-cp314t-macosx_10_15_x86_64.whl", hash = "sha256:8d00f177ca88a77c1cf848d204a38f249751650b601cb6532acc68805d8a8273"}, + {file = "msgpack-1.2.1-cp314-cp314t-macosx_11_0_arm64.whl", hash = "sha256:5bb9c386f0a329c035ddbab4b72d1028bf9627add8dda41070288563d57ed1b1"}, + {file = "msgpack-1.2.1-cp314-cp314t-manylinux2014_aarch64.manylinux_2_17_aarch64.manylinux_2_28_aarch64.whl", hash = "sha256:20466cca18c49c7292a8984bc15d65857b171e7264bdcb5f96baf8be238791fc"}, + {file = "msgpack-1.2.1-cp314-cp314t-manylinux2014_x86_64.manylinux_2_17_x86_64.manylinux_2_28_x86_64.whl", hash = "sha256:196300e7e5d6e74d50f1607ab9c06c4a1484c383cd22defd727902591f7e8dde"}, + {file = "msgpack-1.2.1-cp314-cp314t-manylinux_2_31_riscv64.manylinux_2_39_riscv64.whl", hash = "sha256:575957e79cd51903a4e8495a242442949641e08f1efd5197b43bebd3ea7682b4"}, + {file = "msgpack-1.2.1-cp314-cp314t-musllinux_1_2_aarch64.whl", hash = "sha256:8c2ed1e48cc0f460bf3c7780e7137ff21a4e18433451916f2442c1b21036cd7d"}, + {file = "msgpack-1.2.1-cp314-cp314t-musllinux_1_2_riscv64.whl", hash = "sha256:5f6277e5f783c36786a145e0247fc189a03f35f84b251646e53592d2bc12b355"}, + {file = "msgpack-1.2.1-cp314-cp314t-musllinux_1_2_x86_64.whl", hash = "sha256:f9389552ecf4784886345ead0647e4edc96bee37cbab05b75540f542f766c48c"}, + {file = "msgpack-1.2.1-cp314-cp314t-win32.whl", hash = "sha256:c1c79a604a2969a868a78b6ebd27a887e00c624f14f66b3038e0590cb23332d1"}, + {file = "msgpack-1.2.1-cp314-cp314t-win_amd64.whl", hash = "sha256:f12038a35fabd52e56a3547bab42401af49a45caa6dd00b34c44de235bc93ee2"}, + {file = "msgpack-1.2.1-cp314-cp314t-win_arm64.whl", hash = "sha256:0adcf06ffde0777c0e1a9b771a2b1c4226ba1bbf748c8efcc02fcdeca3299107"}, + {file = "msgpack-1.2.1.tar.gz", hash = "sha256:04c721c2c7448767e9e3f2520a475663d8ee0f09c31890f6d2bd70fd636a9647"}, ] [[package]] @@ -4778,4 +4778,4 @@ propcache = ">=0.2.1" [metadata] lock-version = "2.1" python-versions = "^3.13" -content-hash = "bdff25e5eedd189273905e3f39ef6ab9b8feac6b90a93e3f203b9c8fdbeae8a9" +content-hash = "1a970f19731b802afa0eb5bd83e59674744c43d911440e34890e16a076bbbfa8" diff --git a/pyproject.toml b/pyproject.toml index 94ceeea..9806f76 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -35,6 +35,7 @@ dependencies.humanize = "^4.11.0" dependencies.urllib3 = "^2.6.0" dependencies.werkzeug = "^3.1.4" dependencies.marshmallow = "^4.1.2" +dependencies.msgpack = "^1.2.1" dependencies.quart-schema = "^0.23.0" dependencies.pydantic = "^2.12.5" group.dev.dependencies.Pygments = ">=2.10.0" From ad4c871c52a85a523bac11a2000ad5c8bf96319c Mon Sep 17 00:00:00 2001 From: Jason Antman Date: Sat, 4 Jul 2026 09:35:23 -0400 Subject: [PATCH 5/9] ESB Support: address PR review feedback on the status webhook Adversarial review (claude bot) and Copilot review findings: - (High) Webhook payload last_update was stale for RFID/oops events: MachineState.update() assigned self.last_update = time() *after* the handler ran, but the handler fires the webhook (reading status_dict) before returning, so the payload's last_update lagged one event behind and did not match its own timestamp (contradicting the documented example). Move the last_update assignment before the handler call in the oops and RFID-change branches. Add a regression test asserting payload["last_update"] == payload["timestamp"] for login/logout/oops fired through update(). - (Medium) Fire-and-forget delivery tasks had no strong reference and could be garbage-collected mid-flight (create_task returns a weakly-held task, and _deliver can run ~27s). Track in-flight tasks in a set and discard via a done-callback (the asyncio-docs pattern). - (Low) _deliver opened a new aiohttp.ClientSession per retry attempt; reuse a single session across attempts so retries benefit from connection pooling. - (Copilot) from_env logged the full STATUS_WEBHOOK_URL, which could leak secrets embedded in userinfo/query. Log only scheme+host via a new _safe_url helper; add tests proving credentials are not logged. - (Copilot) test _machine() helper did not return a fresh status_dict per access like the real property; replace the Mock with a small _FakeMachine whose status_dict property returns a fresh dict. All nox sessions pass; webhook.py at 100% coverage. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/dm_mac/models/machine.py | 11 ++++-- src/dm_mac/webhook.py | 54 +++++++++++++++++++++++------- tests/test_webhook.py | 53 +++++++++++++++++++++++++---- tests/views/test_webhook_wiring.py | 29 ++++++++++++++++ 4 files changed, 127 insertions(+), 20 deletions(-) diff --git a/src/dm_mac/models/machine.py b/src/dm_mac/models/machine.py index ff5f08b..9d7b65c 100644 --- a/src/dm_mac/models/machine.py +++ b/src/dm_mac/models/machine.py @@ -966,8 +966,12 @@ async def update( self.internal_temperature_c = internal_temperature_c self.last_checkin = time() if oops: - await self._handle_oops(users) + # Set last_update *before* the handler runs: the handler fires + # the status-change webhook synchronously (from status_dict), so + # last_update must already reflect this event for the webhook + # payload's last_update to match its own timestamp. self.last_update = time() + await self._handle_oops(users) # Handle always-enabled machines - track RFID but maintain always-on state if ( self.machine.always_enabled @@ -983,11 +987,14 @@ async def update( if rfid_value != self.rfid_value: await self._handle_rfid_tracking_always_enabled(users, rfid_value) elif rfid_value != self.rfid_value: + # Set last_update *before* the handler for the same reason as + # the oops branch above: the handler fires the webhook and the + # payload reads last_update from status_dict. + self.last_update = time() if rfid_value is None: await self._handle_rfid_remove() else: await self._handle_rfid_insert(users, rfid_value) - self.last_update = time() else: # No RFID change - check for stale always-enabled state. # This handles the case where always_enabled config was changed diff --git a/src/dm_mac/webhook.py b/src/dm_mac/webhook.py index 4d24295..adfed69 100644 --- a/src/dm_mac/webhook.py +++ b/src/dm_mac/webhook.py @@ -3,6 +3,7 @@ import logging import os from asyncio import CancelledError +from asyncio import Task from asyncio import create_task from asyncio import sleep from time import time @@ -10,6 +11,8 @@ from typing import Any from typing import Dict from typing import Optional +from typing import Set +from urllib.parse import urlsplit import aiohttp @@ -49,6 +52,26 @@ class WebhookNotifier: def __init__(self, url: str): """Initialize a WebhookNotifier posting to ``url``.""" self.url: str = url + #: Strong references to in-flight delivery tasks. ``asyncio`` only + #: holds a *weak* reference to tasks created via + #: :func:`asyncio.create_task`, so without this a long-running + #: :meth:`_deliver` (up to ~27 s of backoff + timeouts) could be + #: garbage-collected mid-flight. Each task removes itself via a + #: done-callback (the pattern from the asyncio docs). + self._tasks: Set["Task[None]"] = set() + + @staticmethod + def _safe_url(url: str) -> str: + """Return ``scheme://host[:port]`` of ``url`` for safe logging. + + Strips any userinfo, path, and query string so credentials embedded + in the URL (userinfo or query params) are never written to logs. + """ + parts = urlsplit(url) + host: str = parts.hostname or "" + if parts.port is not None: + host = f"{host}:{parts.port}" + return f"{parts.scheme}://{host}" if parts.scheme else host @classmethod def from_env(cls) -> Optional["WebhookNotifier"]: @@ -56,7 +79,8 @@ def from_env(cls) -> Optional["WebhookNotifier"]: url: str = os.environ.get("STATUS_WEBHOOK_URL", "").strip() if not url: return None - logger.info("Status-change webhook enabled; posting to %s", url) + # Log only scheme+host: the URL may contain secrets (userinfo/query). + logger.info("Status-change webhook enabled; posting to %s", cls._safe_url(url)) return cls(url) def build_payload( @@ -90,7 +114,11 @@ def notify( ) -> None: """Build the payload and spawn a fire-and-forget delivery task.""" payload: Dict[str, Any] = self.build_payload(machine, event, user=user) - create_task(self._deliver(payload)) + task: "Task[None]" = create_task(self._deliver(payload)) + # Hold a strong reference until the task completes so the event loop's + # weak reference cannot let it be garbage-collected mid-delivery. + self._tasks.add(task) + task.add_done_callback(self._tasks.discard) async def _deliver(self, payload: Dict[str, Any]) -> None: """Deliver one webhook with retries and exponential backoff. @@ -98,25 +126,27 @@ async def _deliver(self, payload: Dict[str, Any]) -> None: Returns as soon as an attempt gets a non-error (< 400) response. On network errors or 4xx/5xx responses it retries up to :data:`MAX_ATTEMPTS` times, backing off exponentially between attempts, - then logs a single error and gives up. + then logs a single error and gives up. A single + :class:`aiohttp.ClientSession` is reused across attempts so retries can + benefit from connection pooling. """ timeout: aiohttp.ClientTimeout = aiohttp.ClientTimeout( total=self.REQUEST_TIMEOUT_SEC ) last_err: Optional[str] = None - for attempt in range(1, self.MAX_ATTEMPTS + 1): - if attempt > 1: - await sleep(self.BACKOFF_BASE_SEC * (2 ** (attempt - 2))) - try: - async with aiohttp.ClientSession(timeout=timeout) as session: + async with aiohttp.ClientSession(timeout=timeout) as session: + for attempt in range(1, self.MAX_ATTEMPTS + 1): + if attempt > 1: + await sleep(self.BACKOFF_BASE_SEC * (2 ** (attempt - 2))) + try: async with session.post(self.url, json=payload) as resp: if resp.status < 400: return last_err = f"HTTP {resp.status}" - except CancelledError: # pragma: no cover - raise - except Exception as ex: - last_err = repr(ex) + except CancelledError: # pragma: no cover + raise + except Exception as ex: + last_err = repr(ex) logger.error( "Failed to deliver status webhook for machine %s event %s after " "%d attempts: %s", diff --git a/tests/test_webhook.py b/tests/test_webhook.py index 7588f62..dd41472 100644 --- a/tests/test_webhook.py +++ b/tests/test_webhook.py @@ -14,8 +14,21 @@ pbm: str = "dm_mac.webhook" -def _machine(status_dict: Optional[Dict[str, Any]] = None) -> Mock: - """Build a fake Machine whose status_dict returns a fresh dict each call.""" +class _FakeMachine: + """Stand-in for Machine whose ``status_dict`` returns a fresh dict each + access, exactly like the real property, so ``build_payload``'s in-place + mutation cannot leak between accesses.""" + + def __init__(self, status_dict: Dict[str, Any]): + self._status_dict = status_dict + + @property + def status_dict(self) -> Dict[str, Any]: + return dict(self._status_dict) + + +def _machine(status_dict: Optional[Dict[str, Any]] = None) -> _FakeMachine: + """Build a fake Machine whose status_dict returns a fresh dict each access.""" if status_dict is None: status_dict = { "name": "planer", @@ -28,10 +41,7 @@ def _machine(status_dict: Optional[Dict[str, Any]] = None) -> Mock: "last_checkin": 100.0, "last_update": 200.0, } - mach = Mock() - # Return a copy each access so build_payload's mutation doesn't leak. - mach.status_dict = dict(status_dict) - return mach + return _FakeMachine(status_dict) class _FakePost: @@ -98,6 +108,37 @@ def test_set_returns_instance(self) -> None: assert notifier is not None assert notifier.url == "https://esb/hook" + def test_does_not_log_secrets(self) -> None: + """from_env logs only scheme+host, never userinfo or query secrets.""" + secret_url = "https://user:pass@esb.example.com/hook?token=SECRET" + with patch.dict("os.environ", {"STATUS_WEBHOOK_URL": secret_url}), patch( + f"{pbm}.logger" + ) as mock_logger: + notifier = WebhookNotifier.from_env() + assert notifier is not None + # The full URL (with credentials) is still used for delivery... + assert notifier.url == secret_url + # ...but the logged message contains only scheme+host. + logged = " ".join(str(a) for a in mock_logger.info.call_args.args) + assert "esb.example.com" in logged + assert "pass" not in logged + assert "SECRET" not in logged + + +class TestSafeUrl: + """Tests for WebhookNotifier._safe_url redaction.""" + + def test_strips_userinfo_and_query(self) -> None: + """Userinfo, path, and query are stripped; scheme+host kept.""" + assert ( + WebhookNotifier._safe_url("https://u:p@host.example:8443/hook?k=v") + == "https://host.example:8443" + ) + + def test_plain_url(self) -> None: + """A plain URL reduces to scheme://host.""" + assert WebhookNotifier._safe_url("https://esb/hook") == "https://esb" + class TestBuildPayload: """Tests for WebhookNotifier.build_payload.""" diff --git a/tests/views/test_webhook_wiring.py b/tests/views/test_webhook_wiring.py index d9c637b..b54019c 100644 --- a/tests/views/test_webhook_wiring.py +++ b/tests/views/test_webhook_wiring.py @@ -6,10 +6,14 @@ from typing import List from unittest.mock import AsyncMock from unittest.mock import Mock +from unittest.mock import patch +from freezegun import freeze_time from quart import Quart from quart.typing import TestClientProtocol +from dm_mac.webhook import WebhookNotifier + from .quart_test_helpers import app_and_client @@ -156,3 +160,28 @@ async def test_disabled_notifier_is_noop(self, tmp_path: Path) -> None: json=_heartbeat("metal-mill", rfid_value="8114346998"), ) assert resp.status_code == 200 + + @freeze_time("2024-01-01 00:00:00") + async def test_payload_last_update_matches_timestamp(self, tmp_path: Path) -> None: + """Regression: the webhook payload's last_update reflects the current + event, so it matches the payload's own timestamp for login/logout/oops + fired through update() (previously it lagged one event behind).""" + app, client = app_and_client(tmp_path) + with patch.object( + WebhookNotifier, "_deliver", new_callable=AsyncMock + ) as mock_deliver: + app.config["WEBHOOK_NOTIFIER"] = WebhookNotifier("http://x") + await client.post( + "/api/machine/update", + json=_heartbeat("metal-mill", rfid_value="8114346998"), + ) # login + await client.post( + "/api/machine/update", json=_heartbeat("metal-mill") + ) # logout + await client.post( + "/api/machine/update", json=_heartbeat("metal-mill", oops=True) + ) # oops + payloads = [call.args[0] for call in mock_deliver.call_args_list] + assert [p["event"] for p in payloads] == ["login", "logout", "oops"] + for p in payloads: + assert p["last_update"] == p["timestamp"], p["event"] From 7d04a8122ef863d02578cdd2429e550a81418977 Mon Sep 17 00:00:00 2001 From: Jason Antman Date: Sat, 4 Jul 2026 10:29:02 -0400 Subject: [PATCH 6/9] ESB Support: bump last_update on all meaningful state changes for webhook MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses a second round of PR review (Copilot inline comments on the control-path/reboot webhooks) — the same last_update staleness class as the earlier adversarial finding, but for the paths the first fix did not cover. The API/Slack-triggered control methods (MachineState.lockout/unlock/oops/ unoops) and _handle_reboot() did not update last_update before firing the status webhook, so lockout/unlock/oops/unoops/reboot events emitted a stale last_update (contradicting the documented "last meaningful state change" semantics and the status_dict schema). Fix by bumping self.last_update = time() at each mutation site: - MachineState.lockout(), unlock(), oops(), unoops() now set last_update inside their locked block (this also covers the MCU oops-button path via _handle_oops -> oops(), so the redundant pre-set previously added in update()'s oops branch is removed). - _handle_reboot() sets last_update (a reboot resets the machine — a meaningful change). - The RFID branch in update() keeps setting last_update before its inline handlers (they have no dedicated mutation method); comment clarified. This makes last_update consistent across all readers (Prometheus, GET /api/machines, and the webhook payload). Updated the three existing tests that asserted the old stale value (lockout/reboot now bump it to "now"), and extended the webhook regression test to assert last_update == timestamp for API oops/unoops/lockout/unlock and reboot. All nox sessions pass; webhook.py at 100% coverage. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/dm_mac/models/machine.py | 30 +++++++++++++++++++-------- tests/views/test_machine.py | 9 +++++--- tests/views/test_webhook_wiring.py | 33 ++++++++++++++++++++++++++++++ 3 files changed, 61 insertions(+), 11 deletions(-) diff --git a/src/dm_mac/models/machine.py b/src/dm_mac/models/machine.py index 9d7b65c..f3852b3 100644 --- a/src/dm_mac/models/machine.py +++ b/src/dm_mac/models/machine.py @@ -827,6 +827,10 @@ async def _handle_reboot(self) -> None: self.machine.display_name, ) # locking handled in update() + # A reboot resets the machine (logs out any user, resets the relay): + # a meaningful state change, so bump last_update before the webhook + # reads it from status_dict. + self.last_update = time() prior_user: Optional[User] = self.current_user self.current_user = None self.is_override_login = False @@ -856,6 +860,9 @@ def lockout(self) -> None: "Machine %s was locked out for maintenance.", self.machine.display_name ) with self._lock: + # Meaningful state change: bump last_update so it is fresh for any + # reader (Prometheus, GET /api/machines, the status webhook payload). + self.last_update = time() self.is_locked_out = True self.relay_desired_state = False self.current_user = None @@ -871,6 +878,9 @@ def unlock(self) -> None: self.machine.display_name, ) with self._lock: + # Meaningful state change: bump last_update so it is fresh for any + # reader (Prometheus, GET /api/machines, the status webhook payload). + self.last_update = time() self.is_locked_out = False self.current_user = None # Restore always-enabled state if applicable @@ -893,6 +903,9 @@ def oops(self, do_locking: bool = True) -> None: ) locker = self._lock if do_locking else nullcontext() with locker: + # Meaningful state change: bump last_update so it is fresh for any + # reader (Prometheus, GET /api/machines, the status webhook payload). + self.last_update = time() self.is_oopsed = True self.relay_desired_state = False self.current_user = None @@ -908,6 +921,9 @@ def unoops(self, do_locking: bool = True) -> None: ) locker = self._lock if do_locking else nullcontext() with locker: + # Meaningful state change: bump last_update so it is fresh for any + # reader (Prometheus, GET /api/machines, the status webhook payload). + self.last_update = time() self.is_oopsed = False self.current_user = None # Restore always-enabled state if applicable @@ -966,11 +982,8 @@ async def update( self.internal_temperature_c = internal_temperature_c self.last_checkin = time() if oops: - # Set last_update *before* the handler runs: the handler fires - # the status-change webhook synchronously (from status_dict), so - # last_update must already reflect this event for the webhook - # payload's last_update to match its own timestamp. - self.last_update = time() + # _handle_oops -> MachineState.oops() bumps last_update itself, + # before it fires the webhook, so no pre-set is needed here. await self._handle_oops(users) # Handle always-enabled machines - track RFID but maintain always-on state if ( @@ -987,9 +1000,10 @@ async def update( if rfid_value != self.rfid_value: await self._handle_rfid_tracking_always_enabled(users, rfid_value) elif rfid_value != self.rfid_value: - # Set last_update *before* the handler for the same reason as - # the oops branch above: the handler fires the webhook and the - # payload reads last_update from status_dict. + # The RFID handlers mutate state inline (no dedicated method + # that bumps last_update), and they fire the status webhook + # before returning. Set last_update first so the webhook payload + # reflects this event rather than the previous one. self.last_update = time() if rfid_value is None: await self._handle_rfid_remove() diff --git a/tests/views/test_machine.py b/tests/views/test_machine.py index 23bac73..26628da 100644 --- a/tests/views/test_machine.py +++ b/tests/views/test_machine.py @@ -814,7 +814,8 @@ async def test_lockout_with_user(self, tmp_path: Path) -> None: assert ms.rfid_present_since == 1689477200.0 assert ms.current_user is None assert ms.relay_desired_state is False - assert ms.last_update == 1689477200.0 + # lockout() bumps last_update (meaningful state change) to "now" + assert ms.last_update == 1689477248.0 assert ms.status_led_rgb == (1.0, 0.5, 0.0) assert ms.status_led_brightness == MachineState.STATUS_LED_BRIGHTNESS @@ -944,7 +945,8 @@ async def test_reboot_without_slack(self, tmp_path: Path) -> None: assert ms.rfid_present_since == 1689477200.0 assert ms.current_user is None assert ms.relay_desired_state is False - assert ms.last_update == 1689477200.0 + # _handle_reboot() bumps last_update (machine reset) to "now" + assert ms.last_update == 1689477248.0 assert ms.status_led_rgb == (0.0, 0.0, 0.0) assert ms.status_led_brightness == 0.0 @@ -1009,7 +1011,8 @@ async def test_reboot_with_slack(self, tmp_path: Path) -> None: assert ms.rfid_present_since == 1689477200.0 assert ms.current_user is None assert ms.relay_desired_state is False - assert ms.last_update == 1689477200.0 + # _handle_reboot() bumps last_update (machine reset) to "now" + assert ms.last_update == 1689477248.0 assert ms.status_led_rgb == (0.0, 0.0, 0.0) assert ms.status_led_brightness == 0.0 assert slack.mock_calls == [call.admin_log("Machine hammer has rebooted.")] diff --git a/tests/views/test_webhook_wiring.py b/tests/views/test_webhook_wiring.py index b54019c..db5e03d 100644 --- a/tests/views/test_webhook_wiring.py +++ b/tests/views/test_webhook_wiring.py @@ -185,3 +185,36 @@ async def test_payload_last_update_matches_timestamp(self, tmp_path: Path) -> No assert [p["event"] for p in payloads] == ["login", "logout", "oops"] for p in payloads: assert p["last_update"] == p["timestamp"], p["event"] + + @freeze_time("2024-01-01 00:00:00") + async def test_control_and_reboot_last_update_matches_timestamp( + self, tmp_path: Path + ) -> None: + """Regression: API oops/lockout/clear and reboot events also carry a + fresh last_update (bumped at the mutation site), matching timestamp.""" + app, client = app_and_client(tmp_path) + with patch.object( + WebhookNotifier, "_deliver", new_callable=AsyncMock + ) as mock_deliver: + app.config["WEBHOOK_NOTIFIER"] = WebhookNotifier("http://x") + await client.post("/api/machine/oops/metal-mill") + await client.delete("/api/machine/oops/metal-mill") + await client.post("/api/machine/locked_out/metal-mill") + await client.delete("/api/machine/locked_out/metal-mill") + # reboot: establish an uptime, then report a lower one + await client.post( + "/api/machine/update", json=_heartbeat("hammer", uptime=100.0) + ) + await client.post( + "/api/machine/update", json=_heartbeat("hammer", uptime=1.0) + ) + payloads = [call.args[0] for call in mock_deliver.call_args_list] + assert [p["event"] for p in payloads] == [ + "oops", + "unoops", + "lockout", + "unlock", + "reboot", + ] + for p in payloads: + assert p["last_update"] == p["timestamp"], p["event"] From a18e86ab639e7fa04c34208ab75c81cb75a1e6ac Mon Sep 17 00:00:00 2001 From: Jason Antman Date: Sat, 4 Jul 2026 10:47:52 -0400 Subject: [PATCH 7/9] ESB Support: address round-3 review (status_dict + always-enabled last_update) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two Copilot inline findings on machine.py: - status_dict read state.current_user with a check-then-use pattern. Capture the user object once into a local before the guard and the field reads, so the serialization can never trip over current_user being reassigned to None. (No real race exists today — status_dict is synchronous with no await, and no thread mutates current_user — but the local is clearer and defensive.) - The always-enabled branch of MachineState.update() bumped last_update on every check-in, so idle always-on machines showed a perpetually-"now" last_update in GET /api/machines and the machine_last_update_timestamp Prometheus gauge (and it contradicted the "last meaningful state change" semantics this PR standardized). Now bump last_update only on an actual change: the transition into the always-on state (relay was off) or a tracked RFID change (login/logout/unknown). Steady-state always-on heartbeats no longer touch last_update. The RFID-change bump still happens before _handle_rfid_tracking_always_enabled fires its webhook, so always-enabled login/logout webhook payloads keep a fresh last_update. Add a regression test asserting the always-on last_update is bumped on first contact and on RFID login, but not on a steady-state heartbeat. All nox sessions pass; 98% coverage. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/dm_mac/models/machine.py | 21 ++++++++--- tests/views/test_machine_always_enabled.py | 41 ++++++++++++++++++++++ 2 files changed, 57 insertions(+), 5 deletions(-) diff --git a/src/dm_mac/models/machine.py b/src/dm_mac/models/machine.py index f3852b3..423b3b1 100644 --- a/src/dm_mac/models/machine.py +++ b/src/dm_mac/models/machine.py @@ -355,11 +355,14 @@ def status_dict(self) -> Dict[str, Any]: ``None`` otherwise. """ state: "MachineState" = self.state + # Capture the user object once so we never check-then-use a field that + # could be reassigned to None between the guard and the reads. + user: Optional[User] = state.current_user current_user: Optional[Dict[str, str]] = None - if state.current_user is not None: + if user is not None: current_user = { - "account_id": state.current_user.account_id, - "full_name": state.current_user.full_name, + "account_id": user.account_id, + "full_name": user.full_name, } return { "name": self.name, @@ -991,13 +994,21 @@ async def update( and not self.is_oopsed and not self.is_locked_out ): + # Only bump last_update on an actual change: the transition + # into the always-on state (relay was off) or a tracked RFID + # change (login/logout/unknown). A steady-state always-on + # heartbeat is not a meaningful update, so it must not keep + # last_update perpetually "now" for /api/machines & Prometheus. + became_on: bool = not self.relay_desired_state + rfid_changed: bool = rfid_value != self.rfid_value self.relay_desired_state = True self.display_text = self.ALWAYS_ON_DISPLAY_TEXT self.status_led_rgb = (0.0, 1.0, 0.0) self.status_led_brightness = self.STATUS_LED_BRIGHTNESS - self.last_update = time() + if became_on or rfid_changed: + self.last_update = time() # Track RFID changes for logging/auditing purposes - if rfid_value != self.rfid_value: + if rfid_changed: await self._handle_rfid_tracking_always_enabled(users, rfid_value) elif rfid_value != self.rfid_value: # The RFID handlers mutate state inline (no dedicated method diff --git a/tests/views/test_machine_always_enabled.py b/tests/views/test_machine_always_enabled.py index dfc93fc..5e82df7 100644 --- a/tests/views/test_machine_always_enabled.py +++ b/tests/views/test_machine_always_enabled.py @@ -296,6 +296,47 @@ async def test_always_enabled_first_contact(self, tmp_path: Path) -> None: "second_relay": False, } + @freeze_time("2023-07-16 03:14:08", tz_offset=0) + async def test_always_enabled_last_update_only_on_change( + self, tmp_path: Path + ) -> None: + """Always-on machines bump last_update only on a real change (transition + into always-on or a tracked RFID change), not on steady-state + heartbeats — so /api/machines and Prometheus don't show a perpetually + "now" last_update for an idle always-on machine.""" + app: Quart + client: TestClientProtocol + app, client = app_and_client(tmp_path) + mname: str = "always-on-machine" + m: Machine = app.config["MACHINES"].machines_by_name[mname] + + def heartbeat(rfid: str = "") -> dict: + return { + "machine_name": mname, + "oops": False, + "rfid_value": rfid, + "uptime": 10.0, + "wifi_signal_db": -54, + "wifi_signal_percent": 92, + "internal_temperature_c": 53.89, + } + + # First contact: relay transitions off -> on, so last_update is set. + await client.post("/api/machine/update", json=heartbeat()) + assert m.state.relay_desired_state is True + assert m.state.last_update == 1689477248.0 + + # Steady-state heartbeat (relay already on, no RFID change): last_update + # must NOT be bumped. A sentinel detects any unwanted write. + m.state.last_update = 111.0 + await client.post("/api/machine/update", json=heartbeat()) + assert m.state.last_update == 111.0 + + # RFID login (tracked change): last_update is bumped to now. + m.state.last_update = 111.0 + await client.post("/api/machine/update", json=heartbeat(rfid="8114346998")) + assert m.state.last_update == 1689477248.0 + @freeze_time("2023-07-16 03:14:08", tz_offset=0) async def test_always_enabled_unlock(self, tmp_path: Path) -> None: """Test always-enabled machine restores always-on state after unlock.""" From 0e577d4c39a193d5c25833c353351567e53ade44 Mon Sep 17 00:00:00 2001 From: Jason Antman Date: Sat, 4 Jul 2026 11:59:08 -0400 Subject: [PATCH 8/9] ESB Support: address round-4 review (docs clarity) Two Copilot documentation findings: - The completed feature doc (docs/features/completed/esb-support.md) still led with the "# Feature Template" heading and the in-progress template instruction line, which is confusing for a finished feature. Retitle it "# ESB Support" (matching the sibling completed docs) with a short note that it captures the original request plus the implementation plan/progress. - The GET /api/machines docs implied last_update is only null when the machine has never checked in, but last_update starts null and is bumped only on a meaningful state change, independent of last_checkin. Clarify in docs/source/http-api.rst that a machine which has only sent idle heartbeats has a last_checkin while last_update is still null, and tighten the MachineStatus.last_update schema description to match. Docs-only; docs build (-W) and all nox sessions pass. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/features/completed/esb-support.md | 6 ++++-- docs/source/http-api.rst | 7 +++++-- src/dm_mac/models/api_schemas.py | 4 +++- 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/docs/features/completed/esb-support.md b/docs/features/completed/esb-support.md index 5872ec7..a7217b3 100644 --- a/docs/features/completed/esb-support.md +++ b/docs/features/completed/esb-support.md @@ -1,6 +1,8 @@ -# Feature Template +# ESB Support -You must read, understand, and follow all instructions in `./README.md` when planning and implementing this feature. +> Status: **completed**. This document is the original feature request plus the +> implementation plan and per-milestone progress notes recorded as the work was +> carried out. ## Overview diff --git a/docs/source/http-api.rst b/docs/source/http-api.rst index f7f8fbb..c493a7f 100644 --- a/docs/source/http-api.rst +++ b/docs/source/http-api.rst @@ -110,8 +110,11 @@ entry has the shape: ``status`` is a derived summary — one of ``locked_out``, ``oops``, ``in_use``, ``idle``, or ``unknown`` (never checked in). ``current_user`` is ``null`` when -no user is logged in. ``last_checkin`` / ``last_update`` are epoch seconds, or -``null`` if the machine has never checked in. +no user is logged in. ``last_checkin`` and ``last_update`` are epoch seconds: +``last_checkin`` is ``null`` until the machine's first check-in, and +``last_update`` is ``null`` until its first *meaningful* state change. These are +independent — a machine that has only sent idle heartbeats has a ``last_checkin`` +while ``last_update`` is still ``null``. .. _http-api.status-webhook: diff --git a/src/dm_mac/models/api_schemas.py b/src/dm_mac/models/api_schemas.py index 1c056ec..b302847 100644 --- a/src/dm_mac/models/api_schemas.py +++ b/src/dm_mac/models/api_schemas.py @@ -94,7 +94,9 @@ class MachineStatus(BaseModel): last_update: Optional[float] = Field( default=None, description="Epoch seconds of the machine's last meaningful state " - "change, or null if none.", + "change, or null until the first such change. Independent of " + "last_checkin: a machine that has only sent idle heartbeats has a " + "last_checkin but a null last_update.", ) From 0f486ab9061ab79e61a572c221e6900c87c3e54e Mon Sep 17 00:00:00 2001 From: Jason Antman Date: Sat, 4 Jul 2026 12:39:09 -0400 Subject: [PATCH 9/9] ESB Support: move msgpack security pin to the dev dependency group Copilot review: msgpack is only pulled in transitively via cachecontrol, which is a dev-group dependency, and it is not used at runtime by the application. Pinning the secure floor under main [dependencies] wrongly expanded the production dependency set (msgpack appeared in the no-dev poetry export). Move `msgpack ^1.2.1` from main dependencies to group.dev.dependencies and regenerate poetry.lock. The dev export (which `nox -s safety` audits via `poetry export --with=dev`) still gets msgpack>=1.2.1, so the GHSA-6v7p-g79w-8964 advisory stays resolved, while the production export no longer contains msgpack. Updated the feature-doc note accordingly. Verified: production export has no msgpack; dev export has msgpack==1.2.1; `nox -s safety` reports no known vulnerabilities; all tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) --- docs/features/completed/esb-support.md | 7 ++++--- poetry.lock | 4 ++-- pyproject.toml | 4 +++- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/docs/features/completed/esb-support.md b/docs/features/completed/esb-support.md index a7217b3..42dca07 100644 --- a/docs/features/completed/esb-support.md +++ b/docs/features/completed/esb-support.md @@ -156,9 +156,10 @@ unit-test coverage (`webhook.py` at 100%). All `nox` sessions pass: `tests`, Note: `nox -s safety` was failing on `main` due to a pre-existing transitive `msgpack==1.2.0` advisory (GHSA-6v7p-g79w-8964), unrelated to this feature. To -satisfy the "all nox sessions passing" criterion, `msgpack ^1.2.1` was pinned -in `pyproject.toml`'s existing "secure versions of transitive dependencies" -block (same mechanism already used for urllib3/werkzeug/marshmallow). +satisfy the "all nox sessions passing" criterion, `msgpack ^1.2.1` is pinned in +`pyproject.toml`. Because `msgpack` is only pulled in via `cachecontrol` (a +dev-group dependency) and is not used at runtime, the pin lives in +`group.dev.dependencies` so it does not expand the production dependency set. * **3.1** Update documentation: ``README.md`` (env var + endpoints if listed), ``docs/source/configuration.rst`` (add ``STATUS_WEBHOOK_URL`` to the env-var diff --git a/poetry.lock b/poetry.lock index 71188d5..f1aacb0 100644 --- a/poetry.lock +++ b/poetry.lock @@ -2277,7 +2277,7 @@ version = "1.2.1" description = "MessagePack serializer" optional = false python-versions = ">=3.10" -groups = ["main", "dev"] +groups = ["dev"] files = [ {file = "msgpack-1.2.1-cp310-cp310-macosx_10_9_x86_64.whl", hash = "sha256:8c7b398c56ff125feae96c2737abfec5595f1fa0aa186df60c56040b8accb95c"}, {file = "msgpack-1.2.1-cp310-cp310-macosx_11_0_arm64.whl", hash = "sha256:1548006a91aa93c5da81f3bdcebc1a0d10cea2d25969754fbe848da622b2b895"}, @@ -4778,4 +4778,4 @@ propcache = ">=0.2.1" [metadata] lock-version = "2.1" python-versions = "^3.13" -content-hash = "1a970f19731b802afa0eb5bd83e59674744c43d911440e34890e16a076bbbfa8" +content-hash = "8836b7bf34b6e306e24834d8494f2ac1f2d155bf7ee7d0f4802135949dc03f94" diff --git a/pyproject.toml b/pyproject.toml index 9806f76..8201cce 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -35,9 +35,11 @@ dependencies.humanize = "^4.11.0" dependencies.urllib3 = "^2.6.0" dependencies.werkzeug = "^3.1.4" dependencies.marshmallow = "^4.1.2" -dependencies.msgpack = "^1.2.1" dependencies.quart-schema = "^0.23.0" dependencies.pydantic = "^2.12.5" +# Security: msgpack is a dev-only transitive (via cachecontrol); pin the secure +# floor in the dev group so it is not added to the production dependency set. +group.dev.dependencies.msgpack = "^1.2.1" group.dev.dependencies.Pygments = ">=2.10.0" group.dev.dependencies.black = ">=21.10b0" group.dev.dependencies.coverage = { extras = [ "toml" ], version = ">=6.2" }