From 28cca721a01303275cf3faf572c660ce947f0916 Mon Sep 17 00:00:00 2001 From: wshallwshall Date: Mon, 27 Jul 2026 10:40:39 -0500 Subject: [PATCH] fix(ci): lint and SAST the same files locally and in CI, and pin the two together The pre-commit hooks were BROADER than CI, which is the dangerous direction. `ruff check` in CI named six explicit paths; the ruff-check hook has no `exclude` and so runs on every changed Python file. bandit in CI scanned `-r messagefoundry tee`; its hook scanned everything but tests/harness/samples. So harness/ (73 findings), tee/ (12), samples/ and docker/ were gated locally and by NOTHING in CI. Touching any file there failed the commit on errors no CI run could report and no green build had ever been able to reveal. I hit it twice in one day: a harness import-sort during an unrelated flake fix, and a one-line UP017 in scripts/ that I reverted rather than drag four unrelated bandit findings into a release PR. A hook LOOSER than CI is an annoyance -- CI still catches it. A hook STRICTER than CI blocks work on a standard the project does not enforce, and that is what makes people reach for `--no-verify` and lose every other gate with it. Scope now has ONE definition per tool instead of being re-stated, differently, in two files: * ruff -- CI runs `ruff check .` (matching `ruff format --check .`, which was ALREADY repo-wide while lint was not). pyproject's [tool.ruff] extend-exclude is the single source of truth. * bandit -- CI runs `-r .` with an --exclude list identical to the hook's, and the hook's exclude was widened to match. Widening surfaced 29 real findings that CI had never been able to see: - 9 in scripts/ (subprocess in claim_check / vuln_metrics / assert_semgrep_handler_taint, one urlopen, two try/except/continue in a hand-run Kerberos lab probe). All are established, already-reviewed patterns -- two of them ALREADY carried `# noqa: S603`/`S310` for ruff's copy of the same rules; bandit does not read noqa, so they now carry `# nosec` as well. - 20 in packaging/messagefoundry-webconsole/tests/, literal test credentials (B105/B106). That is the same "tests carry non-production idioms" case as tests/, one directory deeper, so the exclude on BOTH sides now names it. Recording a mistake worth not repeating: I first measured this as "0 findings, widening is free" and said so. bandit was not installed in the worktree venv, `python -m bandit` failed, and the JSON parse of its error swallowed the failure as an empty result set. The corrected run reports `Total lines of code: 114206` -- a scanned-volume number the broken run never printed, and the thing that distinguishes a clean scan from one that never ran. 99 findings cleared. 88 by safe auto-fix; the rest deliberately, because they were NOT interchangeable: * B905 `zip()` without strict= -- three sites, three different answers, which is exactly why a blanket `strict=False` would have been a fake fix. failover.py and sender.py build both operands from the same source one line above, so strict=True is correct and will catch a future divergence that zip's default would silently truncate. remote.py is the pairwise idiom `zip(x, x[1:])` where the lengths differ BY DESIGN -- strict=True would raise on every non-empty input -- so it became itertools.pairwise, which says "adjacent pairs" outright and drops the slice copy. * SIM105 -- five sites ruff refused to auto-fix, all for the same reason: each carries a comment (`# pragma: no cover`, "underlying socket already deleted", a two-line rationale) that the contextlib.suppress transform would DESTROY. Two more would need a new import purely to satisfy a style preference. Grandfathered per-line with the reason, which is the approach [tool.ruff.lint] already documents for exactly this case. docs/benchmarks/results/ is now ruff-excluded: those are archived measurement ARTIFACTS -- the exact scripts a published number was produced with. Reformatting them edits the record, and a result you cannot reproduce from the script filed beside it is worthless. tests/test_lint_scope_parity.py reads both configurations and compares them, so narrowing one forces narrowing the other. Mutation-verified in BOTH directions: re-narrowing CI's ruff fails it, and adding an `exclude` to the ruff hook fails it. It asserts the CONTRACT, not a particular scope -- widen or narrow freely, provided both sides move together. Full suite green: 8914 passed, 818 skipped. --- .github/workflows/ci.yml | 9 +- .github/workflows/security.yml | 15 ++- .pre-commit-config.yaml | 13 ++- docker/smoke/config/IB_Test_ADT.py | 2 +- docker/smoke/send_adt.py | 5 +- harness/__main__.py | 21 ++-- harness/_login.py | 3 +- harness/acceptance/__main__.py | 2 +- harness/acceptance/matrix.py | 6 +- harness/acceptance/probes.py | 2 +- harness/acceptance/report.py | 2 +- harness/acceptance/runner.py | 2 +- harness/compose.py | 8 +- harness/config/connscale/graph.py | 3 +- harness/config/load/graph.py | 3 +- harness/config/passthrough/graph.py | 3 +- harness/config/shardcert/_shape.py | 3 +- harness/config/shardcert/graph.py | 9 +- harness/config/store_once/graph.py | 3 +- harness/file_panel.py | 6 +- harness/file_transport.py | 2 +- harness/load/connscale/batchbox.py | 6 +- harness/load/connscale/compare.py | 6 +- harness/load/connscale/remote.py | 16 +-- harness/load/connscale/runner.py | 6 +- harness/load/coord.py | 5 +- harness/load/corpus.py | 9 +- harness/load/enginepoll.py | 10 +- harness/load/failover.py | 7 +- harness/load/multishard.py | 10 +- harness/load/report.py | 2 +- harness/load/runner.py | 5 +- harness/load/sender.py | 8 +- harness/load/shardcert.py | 21 ++-- harness/load/shardcert_ladder.py | 8 +- harness/load/sink.py | 9 +- harness/mllp.py | 7 +- harness/monitor.py | 11 +- harness/reconcile/__main__.py | 3 +- harness/reconcile/compare.py | 3 +- harness/reconcile/normalize.py | 4 +- harness/scenarios.py | 6 +- harness/send.py | 6 +- pyproject.toml | 6 + samples/config/IB_DEMO_ORU_handler.py | 4 +- samples/config/IB_PDF_TO_MDM.py | 4 +- samples/config/IB_RTE_ELIGIBILITY.py | 2 +- samples/config/adt.py | 2 +- samples/consistency/validated_adt.py | 2 +- samples/results_relay/results_relay.py | 2 +- samples/send_mllp.py | 5 +- scripts/ci/assert_semgrep_handler_taint.py | 4 +- scripts/hooks/claim_check.py | 2 +- scripts/kerberos_epa_spike.py | 4 +- scripts/security/vuln_metrics.py | 10 +- tee/__main__.py | 11 +- tee/anon/__init__.py | 2 +- tee/mllp.py | 2 +- tee/relay.py | 10 +- tee/store.py | 6 +- tests/test_lint_scope_parity.py | 125 +++++++++++++++++++++ 61 files changed, 319 insertions(+), 174 deletions(-) create mode 100644 tests/test_lint_scope_parity.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 82ec60c..7d95d26 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -163,9 +163,16 @@ jobs: # BOTH platforms on the cheap Linux leg (below). Running them on the 2x-billed Windows legs was # duplicate cost, so gate them to Linux. The Windows legs keep pytest — the one check whose # result genuinely differs by OS (real sockets, ProactorEventLoop, Windows service paths). + # `.` — the WHOLE repo, matching the format check below and the pre-commit ruff hook, which has no + # `exclude` and so lints every changed Python file. This was an explicit allow-list of six paths, + # and the mismatch was not academic: harness/ (73 findings), tee/ (12), samples/ and docker/ were + # linted by the hook and by nothing in CI, so touching any file there blocked the commit on errors + # CI could not see and no CI run could ever have reported. Scope now lives in ONE place — + # pyproject's [tool.ruff] extend-exclude — instead of being re-stated, differently, per tool. + # tests/test_lint_scope_parity.py fails if the two drift apart again. - name: Lint (ruff) if: (needs.changes.outputs.code == 'true' || github.event_name == 'push' || github.event_name == 'workflow_dispatch') && runner.os == 'Linux' - run: ruff check messagefoundry messagefoundry_webconsole tests packaging/messagefoundry-webconsole scripts/webconsole_seam_snapshot.py scripts/docs/backlog_status_check.py scripts/tray/make_icons.py + run: ruff check . - name: Format check (ruff) if: (needs.changes.outputs.code == 'true' || github.event_name == 'push' || github.event_name == 'workflow_dispatch') && runner.os == 'Linux' diff --git a/.github/workflows/security.yml b/.github/workflows/security.yml index 5afd61e..f183943 100644 --- a/.github/workflows/security.yml +++ b/.github/workflows/security.yml @@ -277,7 +277,20 @@ jobs: # `tee/` is in-tree vendored SOUP (a standalone relay) — scanned to the SAME bar as the # engine. Its one urllib GET (tee/mefor_api.py) is per-line `# nosec B310`: the URL scheme is # fixed by operator config (default localhost), never message-derived. - bandit -r messagefoundry tee --skip B101,B110,B311,B404,B608 + # + # SCOPE = the whole repo minus the excludes below, which mirrors the pre-commit bandit hook + # (.pre-commit-config.yaml) EXACTLY. It was `-r messagefoundry tee` while the hook scanned + # everything except tests/harness/samples — so scripts/ (security tooling, subprocess-heavy) + # was gated locally and by nothing in CI. Touching a file there failed the commit on findings + # no CI run could report. Widening cost nothing: the repo is already clean at this bar. + # tests/harness/samples — intentional non-production idioms (asserts, synthetic-data RNG) + # packaging/messagefoundry-webconsole/tests — the same, one directory deeper: 20 B105/B106 + # "hardcoded password" hits that are all literal test credentials + # ide/ — TypeScript; no Python to scan + # docs/benchmarks/results — archived measurement artifacts, not maintained source + # tests/test_lint_scope_parity.py fails if this and the hook drift apart again. + bandit -r . --skip B101,B110,B311,B404,B608 \ + --exclude ./tests,./harness,./samples,./ide,./docs/benchmarks/results,./packaging/messagefoundry-webconsole/tests,./.venv,./node_modules gitleaks: name: gitleaks (secret scan) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 2ae0f49..59d6d67 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -64,11 +64,18 @@ repos: hooks: - id: gitleaks - # Python SAST — same test set + skips as the CI bandit job (security.yml). Engine code only; - # tests/harness/samples carry intentional non-production idioms (asserts, synthetic-data RNG). + # Python SAST — the SAME skips AND the same excluded paths as the CI bandit job (security.yml). + # It said "same ... as CI" before and was not: CI scanned `-r messagefoundry tee` while this hook + # scanned everything but tests/harness/samples, so scripts/ was gated here and by nothing in CI — + # a commit could fail on findings no CI run would ever report. Both sides now name one list, and + # tests/test_lint_scope_parity.py fails if they drift. + # tests/harness/samples — intentional non-production idioms (asserts, synthetic-data RNG) + # packaging/messagefoundry-webconsole/tests — the same, one directory deeper (literal test creds) + # ide/ — TypeScript; no Python to scan + # docs/benchmarks/results — archived measurement artifacts, not maintained source - repo: https://github.com/PyCQA/bandit rev: 1.9.4 hooks: - id: bandit args: ["--skip", "B101,B110,B311,B404,B608"] - exclude: ^(tests/|harness/|samples/) + exclude: ^(tests/|harness/|samples/|ide/|docs/benchmarks/results/|packaging/messagefoundry-webconsole/tests/) diff --git a/docker/smoke/config/IB_Test_ADT.py b/docker/smoke/config/IB_Test_ADT.py index 48a726f..c7e728d 100644 --- a/docker/smoke/config/IB_Test_ADT.py +++ b/docker/smoke/config/IB_Test_ADT.py @@ -11,7 +11,7 @@ reaches a terminal PROCESSED disposition; a non-ADT message would be logged UNROUTED. """ -from messagefoundry import File, MLLP, Send, handler, inbound, outbound, router +from messagefoundry import MLLP, File, Send, handler, inbound, outbound, router inbound("IB_Test_ADT", MLLP(port=2575), router="adt_router") outbound("FILE-OUT_Test_ADT", File(directory="/var/lib/mefor/out/adt", filename="{MSH-10}.hl7")) diff --git a/docker/smoke/send_adt.py b/docker/smoke/send_adt.py index 9b5bedc..1c72fa8 100644 --- a/docker/smoke/send_adt.py +++ b/docker/smoke/send_adt.py @@ -10,6 +10,7 @@ from __future__ import annotations import asyncio +import contextlib import sys from messagefoundry.transports.mllp import MLLPDecoder, frame @@ -37,10 +38,8 @@ async def _send(host: str, port: int, payload: str, timeout: float) -> bytes: return message finally: writer.close() - try: + with contextlib.suppress(OSError): await writer.wait_closed() - except OSError: - pass def main() -> int: diff --git a/harness/__main__.py b/harness/__main__.py index 2e9e8c6..d366601 100644 --- a/harness/__main__.py +++ b/harness/__main__.py @@ -86,6 +86,7 @@ import argparse import sys +from datetime import UTC def main(argv: list[str] | None = None) -> int: @@ -94,7 +95,7 @@ def main(argv: list[str] | None = None) -> int: # CI use. Force UTF-8 on the CLI streams — best-effort, since a pytest/redirect wrapper may # not support reconfigure. for _stream in (sys.stdout, sys.stderr): - try: + try: # noqa: SIM105 - suppress() would add an import to satisfy a pure style preference _stream.reconfigure(encoding="utf-8", errors="replace") # type: ignore[attr-defined] except (AttributeError, ValueError, OSError): pass @@ -268,8 +269,8 @@ def _list_scenarios() -> int: def _run_scenario(name: str, engine_url: str, token: str | None, timeout: float) -> int: - from messagefoundry.apiclient import ApiError, EngineClient from harness.scenarios import SCENARIOS, run_scenario + from messagefoundry.apiclient import ApiError, EngineClient scenario = SCENARIOS.get(name) if scenario is None: @@ -302,11 +303,10 @@ def _run_load(args: argparse.Namespace) -> int: import time from pathlib import Path - from messagefoundry.apiclient import ApiError - from harness.load.profile import LoadProfileError, get_profile from harness.load.report import compare_to_baseline from harness.load.runner import PreflightError, run_load + from messagefoundry.apiclient import ApiError try: profile = get_profile(args.load) @@ -1249,10 +1249,9 @@ def _run_shardcert_driver(argv: list[str]) -> int: import json from pathlib import Path - from messagefoundry.apiclient import ApiError - from harness.load.coord import CoordTimeout, FileDropCoord from harness.load.shardcert import run_shardcert_driver + from messagefoundry.apiclient import ApiError coord = _coord_from_args(args) assert isinstance(coord, FileDropCoord) @@ -1523,10 +1522,9 @@ def _run_shardcert_drive(argv: list[str]) -> int: parser.add_argument("--report-json", help="write the JSON report to this path") args = parser.parse_args(argv) - from messagefoundry.apiclient import ApiError - from harness.load.coord import CoordTimeout, FileDropCoord from harness.load.shardcert import run_shardcert_drive + from messagefoundry.apiclient import ApiError coord = _coord_from_args(args) assert isinstance(coord, FileDropCoord) @@ -1776,7 +1774,7 @@ def _run_shardcert_engine_ladder(argv: list[str]) -> int: def _run_shardcert_drive_ladder(argv: list[str]) -> int: import asyncio - from datetime import datetime, timezone + from datetime import datetime parser = argparse.ArgumentParser( prog="harness shardcert-drive-ladder", @@ -1868,11 +1866,10 @@ def _run_shardcert_drive_ladder(argv: list[str]) -> int: parser.add_argument("--report-json", help="write the consolidated JSON report to this path") args = parser.parse_args(argv) - from messagefoundry.apiclient import ApiError - from harness.load.coord import CoordTimeout, FileDropCoord from harness.load.shardcert import parse_rate_ladder from harness.load.shardcert_ladder import run_drive_ladder + from messagefoundry.apiclient import ApiError try: rates = parse_rate_ladder(args.rate_ladder) @@ -1936,7 +1933,7 @@ def _run_shardcert_drive_ladder(argv: list[str]) -> int: # P2: the raw --rate-ladder string (the invocation's primary independent variable, and the value # that must match the engine box verbatim) so the consolidated JSON is self-describing. "rate_ladder": args.rate_ladder, - "generated_at": datetime.now(timezone.utc).isoformat(), + "generated_at": datetime.now(UTC).isoformat(), "commit_sha": _git_commit_sha(), } # D5 P1: parent-mkdir + never raise on a bad path (a completed run's exit_code must still return). diff --git a/harness/_login.py b/harness/_login.py index fd33a6f..2adeddb 100644 --- a/harness/_login.py +++ b/harness/_login.py @@ -20,9 +20,8 @@ QWidget, ) -from messagefoundry.apiclient import ApiError, EngineClient - from harness._console_widgets import ERROR_COLOR +from messagefoundry.apiclient import ApiError, EngineClient class LoginDialog(QDialog): diff --git a/harness/acceptance/__main__.py b/harness/acceptance/__main__.py index 02d9375..2b2b32d 100644 --- a/harness/acceptance/__main__.py +++ b/harness/acceptance/__main__.py @@ -35,7 +35,7 @@ def main(argv: list[str] | None = None) -> int: for _stream in (sys.stdout, sys.stderr): - try: + try: # noqa: SIM105 - suppress() would add an import to satisfy a pure style preference _stream.reconfigure(encoding="utf-8", errors="replace") # type: ignore[attr-defined] except (AttributeError, ValueError, OSError): pass diff --git a/harness/acceptance/matrix.py b/harness/acceptance/matrix.py index dbe976c..0b39621 100644 --- a/harness/acceptance/matrix.py +++ b/harness/acceptance/matrix.py @@ -23,10 +23,10 @@ from __future__ import annotations from dataclasses import dataclass -from enum import Enum +from enum import StrEnum -class Status(str, Enum): +class Status(StrEnum): """Outcome of a single matrix row. ``str`` mix-in so it serialises as its value.""" PASS = "PASS" @@ -36,7 +36,7 @@ class Status(str, Enum): ERROR = "ERROR" # the check itself broke (bad node id, probe raised) -class Coverage(str, Enum): +class Coverage(StrEnum): """How a row is exercised.""" PROBE = "probe" diff --git a/harness/acceptance/probes.py b/harness/acceptance/probes.py index 4c206ca..96a51df 100644 --- a/harness/acceptance/probes.py +++ b/harness/acceptance/probes.py @@ -18,9 +18,9 @@ import socket import sys import tempfile +from collections.abc import Callable from dataclasses import dataclass from pathlib import Path -from typing import Callable from harness.acceptance.matrix import Status diff --git a/harness/acceptance/report.py b/harness/acceptance/report.py index e86fb01..8989e3e 100644 --- a/harness/acceptance/report.py +++ b/harness/acceptance/report.py @@ -12,8 +12,8 @@ import csv import io from collections import Counter +from collections.abc import Sequence from pathlib import Path -from typing import Sequence from harness._spreadsheet import ( SPREADSHEET_FORMULA_TRIGGERS, diff --git a/harness/acceptance/runner.py b/harness/acceptance/runner.py index 163b81e..5422719 100644 --- a/harness/acceptance/runner.py +++ b/harness/acceptance/runner.py @@ -17,9 +17,9 @@ import subprocess import sys import tempfile +from collections.abc import Callable, Iterable, Sequence from dataclasses import dataclass from pathlib import Path -from typing import Callable, Iterable, Sequence from xml.etree import ElementTree from harness.acceptance.matrix import MATRIX, Coverage, MatrixRow, Status diff --git a/harness/compose.py b/harness/compose.py index 43888c0..3881395 100644 --- a/harness/compose.py +++ b/harness/compose.py @@ -33,11 +33,13 @@ ) from harness._console_widgets import ConfigurableTable -from messagefoundry.generators import _core -from messagefoundry.generators import all_types # noqa: F401 (registers the built-in message types) -from messagefoundry.parsing import HL7PeekError, Peek, normalize from harness.file_transport import DropResult, FileDropWorker from harness.mllp import SendItem, SendResult, SendWorker +from messagefoundry.generators import ( + _core, + all_types, # noqa: F401 (registers the built-in message types) +) +from messagefoundry.parsing import HL7PeekError, Peek, normalize _COLUMNS = ["Time", "Transport", "Result", "Expected", "OK", "Error"] _ACCEPT, _REJECT, _NONE = "Accept (AA/CA)", "Reject (AE/AR)", "No ACK" diff --git a/harness/config/connscale/graph.py b/harness/config/connscale/graph.py index 49632e8..069e052 100644 --- a/harness/config/connscale/graph.py +++ b/harness/config/connscale/graph.py @@ -25,13 +25,12 @@ from __future__ import annotations +from harness.config.connscale._shape import EDIT, ConnScaleShape, load_connscale_shape from messagefoundry import MLLP, Send, handler, inbound, outbound, router from messagefoundry.config.models import RetryPolicy from messagefoundry.config.wiring import HandlerFn, RouterFn from messagefoundry.parsing.message import Message, RawMessage -from harness.config.connscale._shape import EDIT, ConnScaleShape, load_connscale_shape - _SHAPE = load_connscale_shape() # A few attempts with brief backoff: the sink always AA's, so retries shouldn't fire — but a transient diff --git a/harness/config/load/graph.py b/harness/config/load/graph.py index 324ee1a..8f44a4f 100644 --- a/harness/config/load/graph.py +++ b/harness/config/load/graph.py @@ -19,13 +19,12 @@ from __future__ import annotations +from harness.config.load._shape import Shape, apply_transform, load_shape from messagefoundry import MLLP, Send, handler, inbound, outbound, router from messagefoundry.config.models import RetryPolicy from messagefoundry.config.wiring import HandlerFn from messagefoundry.parsing.message import Message, RawMessage -from harness.config.load._shape import Shape, apply_transform, load_shape - _SHAPE = load_shape() # A few attempts with brief backoff: the sink always AA's, so retries shouldn't fire — but a transient diff --git a/harness/config/passthrough/graph.py b/harness/config/passthrough/graph.py index 8dcf668..4c3483a 100644 --- a/harness/config/passthrough/graph.py +++ b/harness/config/passthrough/graph.py @@ -18,12 +18,11 @@ from __future__ import annotations +from harness.config.load._shape import load_shape from messagefoundry import MLLP, PassThrough, Send, handler, inbound, outbound, router from messagefoundry.config.models import RetryPolicy from messagefoundry.parsing.message import Message, RawMessage -from harness.config.load._shape import load_shape - _SHAPE = load_shape() _RETRY = RetryPolicy( diff --git a/harness/config/shardcert/_shape.py b/harness/config/shardcert/_shape.py index ce6f57e..e9f729f 100644 --- a/harness/config/shardcert/_shape.py +++ b/harness/config/shardcert/_shape.py @@ -103,9 +103,8 @@ class B10, in the permissive direction). Split the three, never overload ``dests import os from dataclasses import dataclass -from messagefoundry.parsing.message import Message, RawMessage - from harness.load.ids import SHARDCERT_IDS +from messagefoundry.parsing.message import Message, RawMessage CHEAP = "cheap" EDIT = "edit" diff --git a/harness/config/shardcert/graph.py b/harness/config/shardcert/graph.py index 33c43e6..1ae58cb 100644 --- a/harness/config/shardcert/graph.py +++ b/harness/config/shardcert/graph.py @@ -29,17 +29,16 @@ from __future__ import annotations -from messagefoundry import MLLP, Send, handler, inbound, outbound, router -from messagefoundry.config.models import RetryPolicy -from messagefoundry.config.wiring import HandlerFn, RouterFn -from messagefoundry.parsing.message import Message, RawMessage - from harness.config.shardcert._shape import ( ShardCertShape, apply_transform, load_shape, shared_dest_name, ) +from messagefoundry import MLLP, Send, handler, inbound, outbound, router +from messagefoundry.config.models import RetryPolicy +from messagefoundry.config.wiring import HandlerFn, RouterFn +from messagefoundry.parsing.message import Message, RawMessage _SHAPE = load_shape() diff --git a/harness/config/store_once/graph.py b/harness/config/store_once/graph.py index 176a30c..f129e5d 100644 --- a/harness/config/store_once/graph.py +++ b/harness/config/store_once/graph.py @@ -23,12 +23,11 @@ from __future__ import annotations +from harness.config.load._shape import load_shape from messagefoundry import MLLP, Send, handler, inbound, outbound, router from messagefoundry.config.models import RetryPolicy from messagefoundry.parsing.message import Message, RawMessage -from harness.config.load._shape import load_shape - _SHAPE = load_shape() # A few attempts with brief backoff (the sink always AA's, so retries shouldn't fire) — a transient diff --git a/harness/file_panel.py b/harness/file_panel.py index f8526e4..eba90fb 100644 --- a/harness/file_panel.py +++ b/harness/file_panel.py @@ -30,10 +30,12 @@ ) from harness._console_widgets import ConfigurableTable -from messagefoundry.generators import _core -from messagefoundry.generators import all_types # noqa: F401 (registers the built-in message types) from harness.file_transport import DropResult, FileDropWorker, FolderWatcher from harness.mllp import Received, SendItem +from messagefoundry.generators import ( + _core, + all_types, # noqa: F401 (registers the built-in message types) +) _RANDOM = "(random across all)" _DROP_COLUMNS = ["#", "Type", "Trigger", "Control ID", "File", "Error"] diff --git a/harness/file_transport.py b/harness/file_transport.py index 9019a24..3b5e85c 100644 --- a/harness/file_transport.py +++ b/harness/file_transport.py @@ -21,8 +21,8 @@ from PySide6.QtCore import QFileSystemWatcher, QObject, QTimer, Signal -from messagefoundry.parsing import HL7PeekError, Peek, normalize from harness.mllp import Received, SendItem +from messagefoundry.parsing import HL7PeekError, Peek, normalize _RESCAN_MS = 1000 # safety-net poll; QFileSystemWatcher can miss bursts on some platforms diff --git a/harness/load/connscale/batchbox.py b/harness/load/connscale/batchbox.py index 2d749c3..eda8771 100644 --- a/harness/load/connscale/batchbox.py +++ b/harness/load/connscale/batchbox.py @@ -292,10 +292,8 @@ def _engine_gauge(key: str) -> int: engine_read = _engine_gauge("engine_read") engine_written = _engine_gauge("engine_written") backlog = _engine_gauge("backlog") - achieved_read = max((float(_thr(r, "achieved_aggregate_rate") or 0.0) for r in proc_reports)) - achieved_written = max( - (float(_thr(r, "delivered_aggregate_rate") or 0.0) for r in proc_reports) - ) + achieved_read = max(float(_thr(r, "achieved_aggregate_rate") or 0.0) for r in proc_reports) + achieved_written = max(float(_thr(r, "delivered_aggregate_rate") or 0.0) for r in proc_reports) in_pipeline_peak = max((int(_thr(r, "in_pipeline_peak") or 0) for r in proc_reports), default=0) # A timed-out drain in ANY process (drain_seconds None) poisons the cell's worst-case drain — the diff --git a/harness/load/connscale/compare.py b/harness/load/connscale/compare.py index 5aa090a..36283fa 100644 --- a/harness/load/connscale/compare.py +++ b/harness/load/connscale/compare.py @@ -144,9 +144,7 @@ def ok(self) -> bool: return False if self.collapse_verdict == COLLAPSE_FAIL: return False - if self.throughput_comparable and not self.throughput_ok: - return False - return True + return not (self.throughput_comparable and not self.throughput_ok) @dataclass(frozen=True) @@ -315,7 +313,7 @@ def build_comparison( """Build the per_lane-vs-pooled A/B from a run's records. Returns ``None`` for a single-arm profile (nothing to compare). ``missing_detail`` maps a ``(sweep_mode, count)`` whose pooled arm failed to start to the loud reason (from the runner), surfaced on the missing row.""" - modes = [m for m in claim_modes] + modes = list(claim_modes) if len(modes) < 2: return None baseline_mode = BASELINE_MODE if BASELINE_MODE in modes else modes[0] diff --git a/harness/load/connscale/remote.py b/harness/load/connscale/remote.py index 118bccf..7aa3175 100644 --- a/harness/load/connscale/remote.py +++ b/harness/load/connscale/remote.py @@ -34,15 +34,16 @@ import asyncio import contextlib +import itertools import time from dataclasses import dataclass, field -from datetime import datetime, timezone +from datetime import UTC, datetime from harness.load.connscale.driver import ConnScaleDriver from harness.load.connscale.report import NoLoss from harness.load.connscale.runner import ConnScaleError, _reconcile from harness.load.correlator import Correlator -from harness.load.enginepoll import EngineSample, EnginePoller, sample_until_reconciled +from harness.load.enginepoll import EnginePoller, EngineSample, sample_until_reconciled from harness.load.failover import _await_port from harness.load.ids import ControlIds from harness.load.metrics import Counters, Histogram, LiveMetrics @@ -205,7 +206,10 @@ def check_remote_bands( def _reject_overlaps(blocks: list[tuple[int, int]], label: str) -> None: ordered = sorted(blocks) - for (alo, ahi), (blo, bhi) in zip(ordered, ordered[1:]): + # pairwise, not zip(x, x[1:]): the operands differ in length BY DESIGN here, so this is the one + # B905 site where `strict=True` would be wrong — it would raise on every non-empty input. pairwise + # states "adjacent pairs" directly and drops the throwaway slice copy. + for (alo, ahi), (blo, bhi) in itertools.pairwise(ordered): if blo <= ahi: raise ConnScaleError( f"{label} bands [{alo},{ahi}] and [{blo},{bhi}] overlap — sink/inbound bands per process " @@ -372,7 +376,7 @@ async def run_connscale_remote( def _now_iso() -> str: - return datetime.now(timezone.utc).replace(microsecond=0).isoformat() + return datetime.now(UTC).replace(microsecond=0).isoformat() async def _sample_loop( @@ -383,7 +387,5 @@ async def _sample_loop( sample = await poller.sample_once() if sample is not None: out.append(sample) - try: + with contextlib.suppress(TimeoutError): await asyncio.wait_for(stop.wait(), timeout=interval) - except (asyncio.TimeoutError, TimeoutError): - pass diff --git a/harness/load/connscale/runner.py b/harness/load/connscale/runner.py index 5398fe8..b25405d 100644 --- a/harness/load/connscale/runner.py +++ b/harness/load/connscale/runner.py @@ -54,7 +54,7 @@ ) from harness.load.corpus import Corpus, build_corpus from harness.load.correlator import Correlator -from harness.load.enginepoll import EngineSample, EnginePoller, sample_until_reconciled +from harness.load.enginepoll import EnginePoller, EngineSample, sample_until_reconciled from harness.load.failover import EngineNode, FailoverError, _await_port from harness.load.ids import ControlIds from harness.load.metrics import Counters, Histogram, LiveMetrics @@ -680,10 +680,8 @@ async def _sample_loop( proc = await loop.run_in_executor(None, fd_sampler.sample_proc) _PROC_BY_SAMPLE[id(sample)] = proc out.append(sample) - try: + with contextlib.suppress(TimeoutError): await asyncio.wait_for(stop.wait(), timeout=interval) - except asyncio.TimeoutError: - pass # OS-side process readings (handle count + CPU-seconds + working set) are keyed to the EngineSample diff --git a/harness/load/coord.py b/harness/load/coord.py index 6959094..bee15af 100644 --- a/harness/load/coord.py +++ b/harness/load/coord.py @@ -33,6 +33,7 @@ from __future__ import annotations import asyncio +import contextlib import json import os import time @@ -213,7 +214,5 @@ class CoordTimeout(TimeoutError): def with_missing_ok_unlink(path: Path) -> None: """``Path.unlink(missing_ok=True)`` but tolerant of a concurrent unlink (best-effort cleanup).""" - try: + with contextlib.suppress(OSError): path.unlink(missing_ok=True) - except OSError: - pass diff --git a/harness/load/corpus.py b/harness/load/corpus.py index e811b6a..2e7b6b5 100644 --- a/harness/load/corpus.py +++ b/harness/load/corpus.py @@ -20,12 +20,13 @@ from dataclasses import dataclass from pathlib import Path -from messagefoundry.generators import _core -from messagefoundry.generators import all_types as _all_types # noqa: F401 (registers message types) -from messagefoundry.parsing.message import Message - from harness.load.ids import ControlIds from harness.load.profile import LoadProfile, LoadProfileError, TypeMix +from messagefoundry.generators import _core +from messagefoundry.generators import ( + all_types as _all_types, # noqa: F401 (registers message types) +) +from messagefoundry.parsing.message import Message @dataclass(frozen=True) diff --git a/harness/load/enginepoll.py b/harness/load/enginepoll.py index a1418f7..d2d257a 100644 --- a/harness/load/enginepoll.py +++ b/harness/load/enginepoll.py @@ -23,17 +23,17 @@ import asyncio import time +from collections.abc import Iterable, Mapping, Sequence from dataclasses import dataclass -from typing import Any, Iterable, Mapping, Sequence, TypeVar - -from messagefoundry.apiclient import ApiError, EngineClient +from typing import Any, TypeVar from harness.load.metrics import Counters +from messagefoundry.apiclient import ApiError, EngineClient _T = TypeVar("_T") -def _first_not_none(values: Iterable[_T | None]) -> _T | None: +def _first_not_none[T](values: Iterable[_T | None]) -> _T | None: """The first non-``None`` value, or ``None`` if all are ``None`` (per-process gauges: the connscale harness drives a single engine, so this is exactly that engine's reading).""" for value in values: @@ -568,7 +568,7 @@ async def await_drain(self, *, timeout: float, interval: float) -> float | None: start = loop.time() prev = self.final or await self.sample_once() while loop.time() - start < timeout: - try: + try: # noqa: SIM105 - suppress() would drop the pragma below, which coverage reads await asyncio.wait_for(asyncio.sleep(interval), timeout=interval + 1.0) except TimeoutError: # pragma: no cover - defensive pass diff --git a/harness/load/failover.py b/harness/load/failover.py index cf3b379..3e00f23 100644 --- a/harness/load/failover.py +++ b/harness/load/failover.py @@ -167,7 +167,7 @@ async def stop(self) -> None: proc.terminate() try: await asyncio.wait_for(proc.wait(), timeout=10.0) - except (TimeoutError, asyncio.TimeoutError): + except TimeoutError: with contextlib.suppress(ProcessLookupError): proc.kill() with contextlib.suppress(Exception): @@ -791,7 +791,10 @@ async def _await_single_leader( start = time.perf_counter() while time.perf_counter() - start < timeout: roles = [await n.role(client) for n in nodes] - leaders = [n for n, r in zip(nodes, roles) if r == "primary"] + # strict=True: roles is built one-per-node on the line above, so a mismatch is impossible today + # and would be a bug if it became possible — zip's default silently DROPS the tail, which here + # means skipping nodes in the very scan that decides who the leader is. + leaders = [n for n, r in zip(nodes, roles, strict=True) if r == "primary"] if len(leaders) == 1: return leaders[0] if len(leaders) > 1: diff --git a/harness/load/multishard.py b/harness/load/multishard.py index 4b03363..926721f 100644 --- a/harness/load/multishard.py +++ b/harness/load/multishard.py @@ -40,7 +40,7 @@ import time from collections.abc import Mapping, Sequence from dataclasses import dataclass, field -from datetime import datetime, timezone +from datetime import UTC, datetime from pathlib import Path from harness.load.connscale.driver import ConnScaleDriver @@ -53,7 +53,7 @@ _reconcile, ) from harness.load.correlator import Correlator -from harness.load.enginepoll import EngineSample, EnginePoller, sample_until_reconciled +from harness.load.enginepoll import EnginePoller, EngineSample, sample_until_reconciled from harness.load.failover import EngineNode, _await_port from harness.load.ids import ControlIds from harness.load.metrics import Counters, Histogram, LiveMetrics @@ -784,7 +784,7 @@ def _check_port_layout( def _now_iso() -> str: """Timezone-aware ISO-8601 (UTC, seconds resolution) — the operator's wait-stat window bracket.""" - return datetime.now(timezone.utc).replace(microsecond=0).isoformat() + return datetime.now(UTC).replace(microsecond=0).isoformat() async def _sample_loop( @@ -797,10 +797,8 @@ async def _sample_loop( sample = await poller.sample_once() if sample is not None: out.append(sample) - try: + with contextlib.suppress(TimeoutError): await asyncio.wait_for(stop.wait(), timeout=interval) - except (asyncio.TimeoutError, TimeoutError): - pass def _peak_float(values: list[float | None]) -> float | None: diff --git a/harness/load/report.py b/harness/load/report.py index 652aa80..00cee21 100644 --- a/harness/load/report.py +++ b/harness/load/report.py @@ -291,7 +291,7 @@ def render_console(self) -> str: lines.append( f"engine: peak_backlog={e.peak_backlog} peak_queue_depth={e.peak_queue_depth} " f"dead={e.dead_letters} db_growth={e.db_growth_bytes}B " - f"drain={'%.1fs' % e.drain_seconds if e.drain_seconds is not None else 'TIMEOUT'} " + f"drain={f'{e.drain_seconds:.1f}s' if e.drain_seconds is not None else 'TIMEOUT'} " f"journal={e.journal_mode} synchronous={e.synchronous or 'n/a'} " f"backend={e.db_backend or '?'}" ) diff --git a/harness/load/runner.py b/harness/load/runner.py index dd4b915..e2451fd 100644 --- a/harness/load/runner.py +++ b/harness/load/runner.py @@ -13,9 +13,7 @@ import asyncio import contextlib import time -from typing import Sequence - -from messagefoundry.apiclient import ApiError +from collections.abc import Sequence from harness.load.corpus import build_corpus from harness.load.correlator import Correlator @@ -27,6 +25,7 @@ from harness.load.report import PhaseRecord, RunReport, build_report from harness.load.sender import ConnectionPool, Dispatcher from harness.load.sink import CorrelationSink +from messagefoundry.apiclient import ApiError _STOP_GRACE = 5.0 _SETTLE = 0.25 # let final ACKs/arrivals settle before the final engine sample diff --git a/harness/load/sender.py b/harness/load/sender.py index 888bb7d..6651d94 100644 --- a/harness/load/sender.py +++ b/harness/load/sender.py @@ -22,13 +22,12 @@ from collections import deque from collections.abc import Callable -from messagefoundry.transports.mllp import MLLPDecoder, frame - from harness.load.corpus import Outgoing from harness.load.correlator import Correlator from harness.load.failover_track import FailoverTracker from harness.load.metrics import LiveMetrics from harness.load.profile import Target +from messagefoundry.transports.mllp import MLLPDecoder, frame OnDone = Callable[[], None] _Job = tuple[Outgoing, OnDone | None] @@ -312,7 +311,10 @@ def route(self, code: str) -> ConnectionPool | None: return self._rng.choice(eligible)[1] x = self._rng.random() * total running = 0.0 - for (_target, pool), weight in zip(eligible, weights): + # strict=True: weights is a comprehension over `eligible` just above, so the two are the same + # length by construction. If that ever drifts, zip's default would truncate the weighted draw + # and silently bias target selection rather than fail. + for (_target, pool), weight in zip(eligible, weights, strict=True): running += weight if x <= running: return pool diff --git a/harness/load/shardcert.py b/harness/load/shardcert.py index f1f72a2..0be1544 100644 --- a/harness/load/shardcert.py +++ b/harness/load/shardcert.py @@ -39,12 +39,6 @@ import httpx -from messagefoundry.config.wiring import load_config -from messagefoundry.pipeline.sharding import ( - owned_destination_set, - shard_ids, -) - from harness.config.shardcert._shape import ( BROADCAST, PARTITIONED_FANOUT, @@ -74,6 +68,11 @@ from harness.load.profile import TypeMix, load_profile_text from harness.load.sender import PersistentConnection from harness.load.sink import CorrelationSink +from messagefoundry.config.wiring import load_config +from messagefoundry.pipeline.sharding import ( + owned_destination_set, + shard_ids, +) _CONFIG_DIR = "harness/config/shardcert" @@ -1316,10 +1315,8 @@ async def _sample_in_pipeline_peak( depth = sample.in_pipeline // n_shards if depth > out[0]: out[0] = depth - try: + with contextlib.suppress(TimeoutError): await asyncio.wait_for(stop.wait(), timeout=interval) - except (asyncio.TimeoutError, TimeoutError): - pass finally: await poller.close() @@ -1351,10 +1348,8 @@ async def _sample_in_pipeline_trace( sample = await poller.sample_once() if sample is not None: out.append([round(time.perf_counter() - t0, 3), sample.in_pipeline / n_shards]) - try: + with contextlib.suppress(TimeoutError): await asyncio.wait_for(stop.wait(), timeout=interval) - except (asyncio.TimeoutError, TimeoutError): - pass finally: await poller.close() @@ -3907,7 +3902,7 @@ async def _reap_child(label: str, proc: Any, *, grace: float) -> str: out = b"" try: out, _ = await asyncio.wait_for(proc.communicate(), timeout=grace) - except (asyncio.TimeoutError, TimeoutError): + except TimeoutError: with contextlib.suppress(Exception): proc.kill() with contextlib.suppress(Exception): diff --git a/harness/load/shardcert_ladder.py b/harness/load/shardcert_ladder.py index 9b04afd..7f1b028 100644 --- a/harness/load/shardcert_ladder.py +++ b/harness/load/shardcert_ladder.py @@ -66,6 +66,7 @@ from pathlib import Path from typing import Any +from harness.config.shardcert._shape import BROADCAST from harness.load.coord import ( DRIVE_START, ENGINE_DRAINED, @@ -77,7 +78,6 @@ CoordTimeout, FileDropCoord, ) -from harness.config.shardcert._shape import BROADCAST from harness.load.enginepoll import EMPTY_POOL_STATS, PoolStats from harness.load.shardcert import ( FIDELITY_ACKED_FLOOR, @@ -90,9 +90,9 @@ ShardCertEngineReport, fidelity_note, inbound_band_count, - rung_fidelity, run_shardcert_drive, run_shardcert_engine, + rung_fidelity, ) #: 45M messages/day as the sustained TOTAL message-event rate (inbound + outbound) the ladder pins @@ -354,7 +354,7 @@ class ClaimTiming: #: `stage=`. The blend is retained (its Σn·mean busy-time is exact, and prior reports quote it) but it #: is NOT the outbound claim and must never be read as one. Read `by_stage["outbound"]` for that. #: Empty on a pre-stage log. - by_stage: dict[str, "ClaimTiming"] = field(default_factory=dict) + by_stage: dict[str, ClaimTiming] = field(default_factory=dict) @property def is_empty(self) -> bool: @@ -558,7 +558,7 @@ class EpisodeTiming: ) utilization: float # n-weighted mean (episode + dropped occupancy) / (window × lanes) #: PER-STAGE split — the ONLY correct read for ARM 1. Empty on a log with no episode lines. - by_stage: dict[str, "EpisodeTiming"] = field(default_factory=dict) + by_stage: dict[str, EpisodeTiming] = field(default_factory=dict) @property def is_empty(self) -> bool: diff --git a/harness/load/sink.py b/harness/load/sink.py index edbc824..bf38c74 100644 --- a/harness/load/sink.py +++ b/harness/load/sink.py @@ -24,15 +24,14 @@ import time from collections.abc import Sequence -from messagefoundry.config.models import AckMode -from messagefoundry.parsing import Peek -from messagefoundry.parsing.peek import HL7PeekError -from messagefoundry.transports.mllp import MLLPDecoder, build_ack, frame - from harness.load.correlator import Correlator from harness.load.failover_track import FailoverTracker from harness.load.ids import ControlIds from harness.load.metrics import LiveMetrics +from messagefoundry.config.models import AckMode +from messagefoundry.parsing import Peek +from messagefoundry.parsing.peek import HL7PeekError +from messagefoundry.transports.mllp import MLLPDecoder, build_ack, frame _READ_BYTES = 65536 # larger than the engine's inbound read: the sink only absorbs, never routes diff --git a/harness/mllp.py b/harness/mllp.py index 266af19..e37ae2e 100644 --- a/harness/mllp.py +++ b/harness/mllp.py @@ -10,6 +10,7 @@ from __future__ import annotations +import contextlib import socket import threading import time @@ -111,10 +112,8 @@ def stop(self) -> None: self._stop = True with self._lock: if self._sock is not None: - try: + with contextlib.suppress(OSError): self._sock.shutdown(socket.SHUT_RDWR) - except OSError: - pass def run(self) -> None: delay = 1.0 / self._rate if self._rate > 0 else 0.0 @@ -249,7 +248,7 @@ def _reply(self, sock: QTcpSocket, text: str, control_id: str, seen: int) -> Non def _delayed_aa(self, sock: QTcpSocket, text: str) -> None: if sock not in self._decoders: # the engine may have timed out and closed by now return - try: + try: # noqa: SIM105 - suppress() would drop the reason recorded on the except below self._write_ack(sock, text, "AA", AckMode.ORIGINAL) except RuntimeError: # underlying socket already deleted pass diff --git a/harness/monitor.py b/harness/monitor.py index 5832a7e..925c9ad 100644 --- a/harness/monitor.py +++ b/harness/monitor.py @@ -17,8 +17,9 @@ from __future__ import annotations +import contextlib +from collections.abc import Callable from dataclasses import dataclass -from typing import Callable from PySide6.QtCore import QMetaObject, QObject, Qt, QThread, QTimer, Signal, Slot from PySide6.QtWidgets import ( @@ -35,8 +36,6 @@ QWidget, ) -from messagefoundry.api.models import ConnectionRow, DeadLetterRow -from messagefoundry.apiclient import ApiError, EngineClient from harness._console_widgets import ( ConfigurableTable, MessageDetailPanel, @@ -44,6 +43,8 @@ fmt_ts, ) from harness._login import LoginDialog +from messagefoundry.api.models import ConnectionRow, DeadLetterRow +from messagefoundry.apiclient import ApiError, EngineClient _DEFAULT_URL = "http://127.0.0.1:8765" _POLL_INTERVAL_MS = 1500 @@ -270,10 +271,8 @@ def _ensure_auth(self, client: EngineClient) -> bool: def _disconnect(self) -> None: self._stop_poller() if self._client is not None: - try: + with contextlib.suppress(ApiError): self._client.logout() - except ApiError: - pass self._client.close() self._client = None inner = self._body.widget(1) diff --git a/harness/reconcile/__main__.py b/harness/reconcile/__main__.py index 95c667f..6bb5eeb 100644 --- a/harness/reconcile/__main__.py +++ b/harness/reconcile/__main__.py @@ -27,11 +27,10 @@ import sys from pathlib import Path -from messagefoundry.config.models import AckMode - from harness.reconcile.compare import DEFAULT_KEY, ReconcileResult, load_messages, reconcile from harness.reconcile.normalize import NormalizeRules from harness.reconcile.report import render_json, render_text +from messagefoundry.config.models import AckMode def _parse_field(spec: str) -> tuple[str, int]: diff --git a/harness/reconcile/compare.py b/harness/reconcile/compare.py index fa444e8..2bce321 100644 --- a/harness/reconcile/compare.py +++ b/harness/reconcile/compare.py @@ -21,8 +21,6 @@ from dataclasses import dataclass, field from pathlib import Path -from messagefoundry.parsing.peek import normalize as _normalize_line_endings - from harness.reconcile.normalize import ( Difference, NormalizeRules, @@ -30,6 +28,7 @@ Separators, diff, ) +from messagefoundry.parsing.peek import normalize as _normalize_line_endings #: Default per-connection match key — the message control id (``MSH-10``). DEFAULT_KEY: tuple[str, int] = ("MSH", 10) diff --git a/harness/reconcile/normalize.py b/harness/reconcile/normalize.py index c8f4200..d556e9c 100644 --- a/harness/reconcile/normalize.py +++ b/harness/reconcile/normalize.py @@ -42,7 +42,7 @@ class Separators: subcomponent: str @classmethod - def from_message(cls, message: str) -> "Separators": + def from_message(cls, message: str) -> Separators: """Read the separators from the ``MSH`` header (``MSH-1`` = field sep, ``MSH-2`` = enc chars).""" norm = _normalize_line_endings(message) if not norm.startswith("MSH") or len(norm) < 5: @@ -78,7 +78,7 @@ class NormalizeRules: sort_segments: frozenset[str] = frozenset() ignore_segments: frozenset[str] = frozenset() - def with_blanks(self, *fields: tuple[str, int]) -> "NormalizeRules": + def with_blanks(self, *fields: tuple[str, int]) -> NormalizeRules: """Return a copy with extra blanked fields added (keeps the defaults).""" return NormalizeRules( blank_fields=self.blank_fields | frozenset(fields), diff --git a/harness/scenarios.py b/harness/scenarios.py index c814095..ebc163b 100644 --- a/harness/scenarios.py +++ b/harness/scenarios.py @@ -20,8 +20,10 @@ from dataclasses import dataclass from messagefoundry.apiclient import ApiError, EngineClient -from messagefoundry.generators import _core -from messagefoundry.generators import all_types # noqa: F401 (registers the built-in message types) +from messagefoundry.generators import ( + _core, + all_types, # noqa: F401 (registers the built-in message types) +) from messagefoundry.transports.mllp import MLLPDecoder, frame _TERMINAL = {"processed", "unrouted", "filtered", "error"} diff --git a/harness/send.py b/harness/send.py index 421c4fa..073780a 100644 --- a/harness/send.py +++ b/harness/send.py @@ -22,9 +22,11 @@ ) from harness._console_widgets import ConfigurableTable -from messagefoundry.generators import _core -from messagefoundry.generators import all_types # noqa: F401 (registers the built-in message types) from harness.mllp import SendItem, SendResult, SendWorker +from messagefoundry.generators import ( + _core, + all_types, # noqa: F401 (registers the built-in message types) +) _RANDOM = "(random across all)" _COLUMNS = ["#", "Type", "Trigger", "Control ID", "ACK", "Latency (ms)", "Error"] diff --git a/pyproject.toml b/pyproject.toml index 710c9cc..b217fc4 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -236,6 +236,12 @@ line-length = 100 # this to py314 if/when we choose to adopt the unparenthesized style in a dedicated formatting pass. target-version = "py313" +# Archived benchmark ARTIFACTS, not maintained source. docs/benchmarks/results/-/ holds +# the exact scripts a published measurement was produced with; reformatting them edits the record, and +# a number you cannot reproduce from the script filed beside it is worthless. New benchmark code lives +# outside results/ and is linted normally. `extend-exclude` (not `exclude`) so ruff's defaults survive. +extend-exclude = ["docs/benchmarks/results"] + [tool.ruff.lint] # Signal 10 (Code Quality & Anti-Slop rubric, docs/Code_Quality_Standards.md): broaden ruff beyond its # E/F defaults to the AI-slop-adjacent families -- flake8-bugbear (B), comprehensions (C4), simplify diff --git a/samples/config/IB_DEMO_ORU_handler.py b/samples/config/IB_DEMO_ORU_handler.py index ed0237a..7527ee4 100644 --- a/samples/config/IB_DEMO_ORU_handler.py +++ b/samples/config/IB_DEMO_ORU_handler.py @@ -9,10 +9,10 @@ small, reviewable, and unit-testable — not inlined here. """ -from messagefoundry import Send, handler - from _demo_oru_transforms import apply_demo_oru_transforms +from messagefoundry import Send, handler + # The outbound is declared as data in connections.toml; the Handler references it by name. OB_DEMO_ORU = "OB_DEMO_ORU" diff --git a/samples/config/IB_PDF_TO_MDM.py b/samples/config/IB_PDF_TO_MDM.py index ae4cebe..a2eca81 100644 --- a/samples/config/IB_PDF_TO_MDM.py +++ b/samples/config/IB_PDF_TO_MDM.py @@ -17,6 +17,8 @@ Drop a synthetic (never real PHI) PDF into the DEV inbox to exercise it. Endpoints are loopback/synthetic. """ +from _pdf_mdm_transforms import build_mdm_from_pdf + from messagefoundry import ( MLLP, ContentType, @@ -29,8 +31,6 @@ router, ) -from _pdf_mdm_transforms import build_mdm_from_pdf - # File inbound: poll a DEV drop directory for *.pdf and deliver each as raw bytes (content_type=BINARY → # base64-carried, routed as a RawMessage). The bind directory is a DEV path; a real deployment points it # at the partner's landing zone. diff --git a/samples/config/IB_RTE_ELIGIBILITY.py b/samples/config/IB_RTE_ELIGIBILITY.py index d61e8f5..a3d7c01 100644 --- a/samples/config/IB_RTE_ELIGIBILITY.py +++ b/samples/config/IB_RTE_ELIGIBILITY.py @@ -20,11 +20,11 @@ """ from messagefoundry import ( + X12, AckMode, ContentType, Loopback, Send, - X12, env, handler, inbound, diff --git a/samples/config/adt.py b/samples/config/adt.py index cbe718a..7a14eed 100644 --- a/samples/config/adt.py +++ b/samples/config/adt.py @@ -23,7 +23,7 @@ inside the handler works just as well — the engine/dry-run keeps the set active while a handler runs.) """ -from messagefoundry import File, MLLP, Send, code_set, handler, inbound, outbound, router +from messagefoundry import MLLP, File, Send, code_set, handler, inbound, outbound, router inbound("IB_Test_ADT", MLLP(port=2575), router="adt_router") outbound("FILE-OUT_Test_ADT", File(directory="./out/adt", filename="{MSH-10}.hl7")) diff --git a/samples/consistency/validated_adt.py b/samples/consistency/validated_adt.py index 27c6608..4c5564e 100644 --- a/samples/consistency/validated_adt.py +++ b/samples/consistency/validated_adt.py @@ -23,7 +23,7 @@ python -m messagefoundry serve --config samples/consistency --db ./mf.db --env dev """ -from messagefoundry import File, MLLP, Send, handler, inbound, outbound, router +from messagefoundry import MLLP, File, Send, handler, inbound, outbound, router from messagefoundry.parsing.consistency import ( ConsistencyError, check, diff --git a/samples/results_relay/results_relay.py b/samples/results_relay/results_relay.py index 91558e0..1348e90 100644 --- a/samples/results_relay/results_relay.py +++ b/samples/results_relay/results_relay.py @@ -26,7 +26,7 @@ --messages samples/results_relay/messages --show-phi """ -from messagefoundry import File, MLLP, Send, code_set, env, handler, inbound, outbound, router +from messagefoundry import MLLP, File, Send, code_set, env, handler, inbound, outbound, router from messagefoundry.parsing.message import Message OB_EHR = "OB_EHR_ORU" diff --git a/samples/send_mllp.py b/samples/send_mllp.py index 1fb919d..a6d91f0 100644 --- a/samples/send_mllp.py +++ b/samples/send_mllp.py @@ -16,6 +16,7 @@ import argparse import asyncio +import contextlib from pathlib import Path from messagefoundry.parsing import normalize @@ -36,10 +37,8 @@ async def _send(host: str, port: int, payload: str, timeout: float) -> bytes: return message finally: writer.close() - try: + with contextlib.suppress(OSError): await writer.wait_closed() - except OSError: - pass def main(argv: list[str] | None = None) -> int: diff --git a/scripts/ci/assert_semgrep_handler_taint.py b/scripts/ci/assert_semgrep_handler_taint.py index 823c0d3..fc9ae46 100644 --- a/scripts/ci/assert_semgrep_handler_taint.py +++ b/scripts/ci/assert_semgrep_handler_taint.py @@ -42,7 +42,9 @@ def main() -> int: # Scan a copy in a temp dir (outside the tests tree, not a git repo) so it is actually analyzed. with tempfile.TemporaryDirectory() as tmp: shutil.copy(_FIXTURE, Path(tmp) / _FIXTURE.name) - proc = subprocess.run( # noqa: S603 — fixed args, no shell + # nosec B603 B607 - fixed argv, no shell. The `noqa` covers ruff's copy of these rules; bandit + # does not read noqa, so both annotations are needed now that CI scans scripts/ too. + proc = subprocess.run( # noqa: S603 — fixed args, no shell # nosec B603 B607 ["semgrep", "--config", str(_RULES), "--json", "--metrics", "off", tmp], # noqa: S607 capture_output=True, text=True, diff --git a/scripts/hooks/claim_check.py b/scripts/hooks/claim_check.py index bb9f731..f3a8195 100644 --- a/scripts/hooks/claim_check.py +++ b/scripts/hooks/claim_check.py @@ -48,7 +48,7 @@ def _git(*args: str) -> str: - return subprocess.run( + return subprocess.run( # nosec B603 B607 - fixed argv, no shell, no caller-supplied executable ["git", *args], capture_output=True, text=True, encoding="utf-8", errors="replace" ).stdout diff --git a/scripts/kerberos_epa_spike.py b/scripts/kerberos_epa_spike.py index c8aa27d..b64a661 100644 --- a/scripts/kerberos_epa_spike.py +++ b/scripts/kerberos_epa_spike.py @@ -148,7 +148,7 @@ def _kerberos_capable() -> bool: proxy = getattr(importlib.import_module(mod), cls) if "kerberos" in proxy.available_protocols(): return True - except Exception: + except Exception: # nosec B112 - probing candidate SSPI providers; absent is the norm continue return False except Exception: @@ -169,7 +169,7 @@ def _detect_provider() -> str: proxy = getattr(importlib.import_module(mod), cls) if "kerberos" in proxy.available_protocols(): return label - except Exception: + except Exception: # nosec B112 - probing candidate SSPI providers; absent is the norm continue return "pure-Python NTLM fallback (NOT Kerberos-capable)" diff --git a/scripts/security/vuln_metrics.py b/scripts/security/vuln_metrics.py index 8dac1ac..80fbf4b 100644 --- a/scripts/security/vuln_metrics.py +++ b/scripts/security/vuln_metrics.py @@ -33,7 +33,7 @@ import sys import urllib.error import urllib.request -from datetime import datetime, timezone +from datetime import UTC, datetime from pathlib import Path from typing import Any @@ -46,7 +46,7 @@ def _gh(args: list[str]) -> Any: """Run a `gh` command that emits JSON; return the parsed value ([] on failure).""" try: - out = subprocess.run( + out = subprocess.run( # nosec B603 B607 - fixed argv (`gh`), no shell, args are literals ["gh", *args], capture_output=True, text=True, @@ -67,7 +67,9 @@ def _gh(args: list[str]) -> Any: def _http_json(url: str, timeout: float) -> Any: req = urllib.request.Request(url, headers={"User-Agent": _USER_AGENT}) try: - with urllib.request.urlopen(req, timeout=timeout) as resp: # noqa: S310 - fixed https feeds + # nosec B310 - same reason as the noqa: the URL is a fixed https advisory feed, never + # message-derived or caller-supplied, so no file:/custom scheme is reachable here. + with urllib.request.urlopen(req, timeout=timeout) as resp: # noqa: S310 # nosec B310 return json.loads(resp.read()) except (urllib.error.URLError, TimeoutError, OSError, json.JSONDecodeError) as exc: print(f"warning: GET {url} failed: {exc}", file=sys.stderr) @@ -280,7 +282,7 @@ def main(argv: list[str] | None = None) -> int: p.add_argument("--reachability", default=None, help="triaged reachable/total, e.g. 3/5") args = p.parse_args(argv) - now = _parse_dt(args.now) or datetime.now(timezone.utc) + now = _parse_dt(args.now) or datetime.now(UTC) adopter_repos = [r.strip() for r in args.adopter_repos.split(",") if r.strip()] row = compute(args.repo, args.limit, args.timeout, now, adopter_repos, args.reachability) diff --git a/tee/__main__.py b/tee/__main__.py index 1be39d7..e45352b 100644 --- a/tee/__main__.py +++ b/tee/__main__.py @@ -32,6 +32,7 @@ import argparse import asyncio +import contextlib import json import logging import os @@ -39,7 +40,7 @@ import ssl import sys import time -from datetime import datetime, timezone +from datetime import UTC, datetime from pathlib import Path from tee import __version__, mefor_api @@ -113,7 +114,7 @@ def _parse_age(spec: str) -> float: seconds = int(match.group(1)) * {"d": 86400, "h": 3600, "m": 60}[match.group(2)] return time.time() - seconds try: - day = datetime.strptime(spec, "%Y-%m-%d").replace(tzinfo=timezone.utc) + day = datetime.strptime(spec, "%Y-%m-%d").replace(tzinfo=UTC) except ValueError: raise argparse.ArgumentTypeError( f"expected an age like '7d'/'12h'/'30m' or a UTC date 'YYYY-MM-DD', got {spec!r}" @@ -336,10 +337,8 @@ async def _run(args: argparse.Namespace) -> int: capture_corepoint_copy=args.capture_corepoint_copy, ) relay = TeeRelay(config) - try: + with contextlib.suppress(asyncio.CancelledError): await relay.serve_forever() - except asyncio.CancelledError: - pass return 0 @@ -353,7 +352,7 @@ async def _naks(args: argparse.Namespace) -> int: print("no NAKs or transport errors logged") return 0 for row in rows: - when = datetime.fromtimestamp(row.at, tz=timezone.utc).strftime("%Y-%m-%dT%H:%M:%SZ") + when = datetime.fromtimestamp(row.at, tz=UTC).strftime("%Y-%m-%dT%H:%M:%SZ") code = row.ack_code or row.outcome print( f"{when} {row.direction:<17} {row.leg:<9} {code:<6} " diff --git a/tee/anon/__init__.py b/tee/anon/__init__.py index 7c72ba1..51bd77a 100644 --- a/tee/anon/__init__.py +++ b/tee/anon/__init__.py @@ -23,7 +23,7 @@ from .hl7 import anonymize_message from .keying import Keyer from .leak import leak_check -from .rules import AnonError, DEFAULT_RULES, FieldRule, RuleError, SurrogateKind, load_rules +from .rules import DEFAULT_RULES, AnonError, FieldRule, RuleError, SurrogateKind, load_rules __all__ = [ "DEFAULT_RULES", diff --git a/tee/mllp.py b/tee/mllp.py index ccac362..a55d651 100644 --- a/tee/mllp.py +++ b/tee/mllp.py @@ -15,8 +15,8 @@ from __future__ import annotations import re +from collections.abc import Iterator from datetime import datetime -from typing import Iterator __all__ = [ "SB", diff --git a/tee/relay.py b/tee/relay.py index 752dd03..de23105 100644 --- a/tee/relay.py +++ b/tee/relay.py @@ -28,9 +28,9 @@ import asyncio import logging +from collections.abc import AsyncIterator from contextlib import suppress from dataclasses import dataclass -from typing import AsyncIterator from tee import mllp from tee.store import RelayStore @@ -113,14 +113,14 @@ async def _forward_once( reader, writer = await asyncio.wait_for( asyncio.open_connection(host, port), connect_timeout ) - except (OSError, asyncio.TimeoutError) as exc: + except (TimeoutError, OSError) as exc: raise ForwardError(f"connect to {host}:{port} failed: {exc}") from exc try: writer.write(mllp.frame(payload)) await asyncio.wait_for(writer.drain(), send_timeout) ack = await asyncio.wait_for(_read_one_frame(reader, max_frame_bytes), send_timeout) - except asyncio.TimeoutError as exc: + except TimeoutError as exc: raise ForwardError(f"timed out talking to {host}:{port}") from exc except (OSError, mllp.FrameError) as exc: raise ForwardError(f"I/O error talking to {host}:{port}: {exc}") from exc @@ -196,7 +196,7 @@ async def _send_ack(self, writer: asyncio.StreamWriter, message: bytes) -> bool: writer.write(mllp.frame(mllp.build_ack(message))) await asyncio.wait_for(writer.drain(), self.config.send_timeout) return True - except (asyncio.TimeoutError, OSError) as exc: + except (TimeoutError, OSError) as exc: logger.warning("failed to ACK a peer (connection slow or closed): %r", exc) return False @@ -509,7 +509,7 @@ async def _iter_frames(self, reader: asyncio.StreamReader) -> AsyncIterator[byte chunk = await asyncio.wait_for(reader.read(_READ_CHUNK), cfg.receive_timeout) else: chunk = await reader.read(_READ_CHUNK) - except asyncio.TimeoutError: + except TimeoutError: raise _ConnectionClosed from None except OSError: raise _ConnectionClosed from None diff --git a/tee/store.py b/tee/store.py index 55c889d..9bb3e1c 100644 --- a/tee/store.py +++ b/tee/store.py @@ -19,7 +19,7 @@ import time from collections import Counter from dataclasses import dataclass -from datetime import datetime, timezone +from datetime import UTC, datetime from typing import Any import aiosqlite @@ -95,7 +95,7 @@ def _clean_detail(detail: str | None) -> str | None: def _iso(at: float) -> str: """An epoch-seconds timestamp as a UTC ISO-8601 string (for human/AI-readable export).""" - return datetime.fromtimestamp(at, tz=timezone.utc).strftime("%Y-%m-%dT%H:%M:%SZ") + return datetime.fromtimestamp(at, tz=UTC).strftime("%Y-%m-%dT%H:%M:%SZ") def _is_nak(row: aiosqlite.Row) -> bool: @@ -166,7 +166,7 @@ def _secure_file(path: str) -> None: """Best-effort owner-only permissions on the DB file **and its WAL/SHM siblings** (which WAL mode creates and which can hold message data when body capture is on).""" for sibling in (path, path + "-wal", path + "-shm"): - try: + try: # noqa: SIM105 - suppress() would drop the rationale recorded in the handler below os.chmod(sibling, 0o600) except OSError: # Non-fatal: on some filesystems / Windows ACLs chmod is a partial no-op. Deployments that diff --git a/tests/test_lint_scope_parity.py b/tests/test_lint_scope_parity.py new file mode 100644 index 0000000..d5626b9 --- /dev/null +++ b/tests/test_lint_scope_parity.py @@ -0,0 +1,125 @@ +# SPDX-License-Identifier: AGPL-3.0-or-later +# Copyright (C) 2026 MessageFoundry Organization and contributors +"""The pre-commit hooks and the CI gates must lint/scan the SAME files. + +They did not, and the divergence was silent in the worst direction: the hooks were BROADER than CI. +`ruff check` in CI named six explicit paths while the pre-commit ruff hook has no `exclude` and so +runs on every changed Python file; bandit in CI scanned `-r messagefoundry tee` while its hook scanned +everything but tests/harness/samples. The result was 99 ruff findings in harness/, tee/, samples/ and +docker/ that were gated locally and by nothing in CI — so touching any file there blocked the commit +on errors that no CI run could report, and that no green build had ever been able to reveal. + +The direction matters. A hook LOOSER than CI is an annoyance: CI still catches it. A hook STRICTER +than CI is a trap — it blocks work on a standard the project does not actually enforce, which is +what makes people reach for `--no-verify` and lose every other gate with it. + +These tests read both configurations and compare them, so the next person to narrow one has to +narrow the other. They assert the CONTRACT, not any particular scope: widen or narrow freely, +provided both sides move together. +""" + +from __future__ import annotations + +import re +from pathlib import Path +from typing import Any + +import pytest + +yaml = pytest.importorskip("yaml") + +_ROOT = Path(__file__).resolve().parents[1] +_PRECOMMIT = _ROOT / ".pre-commit-config.yaml" +_CI = _ROOT / ".github" / "workflows" / "ci.yml" +_SECURITY = _ROOT / ".github" / "workflows" / "security.yml" + + +def _hooks() -> dict[str, dict[str, Any]]: + cfg = yaml.safe_load(_PRECOMMIT.read_text(encoding="utf-8")) + return {h["id"]: h for repo in cfg["repos"] for h in repo["hooks"]} + + +def _ci_step_run(workflow: Path, name_fragment: str) -> str: + """The `run:` body of the first step whose name contains ``name_fragment``.""" + wf = yaml.safe_load(workflow.read_text(encoding="utf-8")) + for job in wf["jobs"].values(): + for step in job.get("steps") or []: + if name_fragment.lower() in str(step.get("name", "")).lower() and "run" in step: + return str(step["run"]) + raise AssertionError(f"no step matching {name_fragment!r} in {workflow.name}") + + +def test_ruff_lints_the_whole_repo_on_both_sides() -> None: + """The hook has no `exclude`, so it lints every changed Python file. CI must do the same. + + An allow-list in CI cannot be kept in step with "everything" by hand — it was six paths while the + hook covered the repo. `ruff check .` makes pyproject's [tool.ruff] extend-exclude the SINGLE + definition of scope, which is the only arrangement that cannot drift. + """ + hook = _hooks()["ruff-check"] + assert "exclude" not in hook, ( + "the ruff-check hook grew an `exclude` — CI runs `ruff check .` over the whole repo, so an " + "exclude here makes the hook quietly weaker than the gate it mirrors. Put the exclusion in " + "pyproject [tool.ruff] extend-exclude, where BOTH sides read it." + ) + run = _ci_step_run(_CI, "Lint (ruff)") + assert re.search(r"ruff check\s+\.\s*$", run.strip()), ( + f"CI must lint the whole repo (`ruff check .`) to match the hook; got: {run.strip()!r}" + ) + + +def test_ruff_format_and_lint_agree_on_scope() -> None: + """Format was already repo-wide (`ruff format --check .`) while lint was an allow-list. Two ruff + invocations disagreeing about which files are 'the project' is the same bug in miniature.""" + lint = _ci_step_run(_CI, "Lint (ruff)").strip() + fmt = _ci_step_run(_CI, "Format check (ruff)").strip() + assert lint.endswith("."), lint + assert fmt.endswith("."), fmt + + +def _bandit_skips(text: str) -> set[str]: + m = re.search(r"--skip[= ]([A-Z0-9,]+)", text) + assert m, f"no --skip found in: {text[:200]!r}" + return set(m.group(1).split(",")) + + +def test_bandit_skips_match() -> None: + hook_args = " ".join(_hooks()["bandit"].get("args") or []) + ci = _ci_step_run(_SECURITY, "Scan source for insecure patterns") + assert _bandit_skips(hook_args) == _bandit_skips(ci), ( + "the bandit hook and the CI bandit job disagree about which tests are skipped — one of them " + "is enforcing a standard the other does not" + ) + + +def test_bandit_excludes_match() -> None: + """The hook excludes by regex, CI by comma-separated paths. Compare the PATHS they name. + + CI additionally excludes .venv/node_modules, which pre-commit never passes to a hook (they are + untracked), so those are ignored rather than required on both sides. + """ + hook_re = _hooks()["bandit"]["exclude"] + hook_paths = {p.strip("/") for p in re.findall(r"[\w./-]+/", hook_re)} + + ci = _ci_step_run(_SECURITY, "Scan source for insecure patterns") + m = re.search(r"--exclude[= ]([^\s\\]+)", ci) + assert m, "CI bandit step has no --exclude" + # removeprefix, not strip("./"): strip() removes those CHARACTERS from both ends, so "./.venv" + # would become "venv" and silently fail to match the untracked set below. + untracked = {".venv", "node_modules"} + ci_paths = {p.strip().removeprefix("./").rstrip("/") for p in m.group(1).split(",")} - untracked + + assert hook_paths == ci_paths, ( + f"bandit scope drifted: hook excludes {sorted(hook_paths)}, CI excludes {sorted(ci_paths)}. " + "A path excluded on ONE side only is scanned by one gate and not the other — which is how " + "scripts/ came to be gated locally and by nothing in CI." + ) + + +def test_ci_bandit_scans_the_repo_not_an_allow_list() -> None: + """`-r messagefoundry tee` silently stopped covering scripts/ the moment security tooling moved + there. Scanning `.` minus explicit excludes cannot go stale that way.""" + ci = _ci_step_run(_SECURITY, "Scan source for insecure patterns") + assert re.search(r"bandit\s+-r\s+\.", ci), ( + f"CI bandit must scan `-r .` with explicit --exclude, not an allow-list of dirs; got: {ci!r}" + )