From e2ee1d7feed6b644b76dd080f0401825987ed5de Mon Sep 17 00:00:00 2001 From: JC-000 <3798556+JC-000@users.noreply.github.com> Date: Mon, 7 Sep 2026 23:02:32 -0500 Subject: [PATCH 1/4] feat(net): assert the fleet net_last_error registry at build time (#184) The ip65 ($40-$7F) and UCI ($80-$BF) error families are one namespace each, shared with c64-wireguard. #185 wrote their allocations into our two headers as prose. Prose does not fail a build, and the fleet has already had two collisions -- $88 (live four days) and wg#120's first commit minting $40-$44 over our $41-$45, caught only by a human reviewer. Two guards, one for each other's blind spot: src/net_err_registry_asserts.s -- picked up by the Makefile's $(wildcard src/*.s), so it assembles into every build, both backends, every profile, and emits no bytes (equates and .assert only; PRG sha256 unchanged on all five profiles). It carries the peer's codes as NET_ERR_PEER_* equates -- a namespace that is obviously not ours, so the #185 rule that an equate in our headers would read as "we emit this" still holds -- and asserts that no code of ours lands on one, that each is in family range, and that no published value has been reassigned. Scope `error`, not `lderror`: every operand is a local equate, so ca65 settles it at the offending object rather than at the link. tools/test_net_err_registry.py -- pure logic, milliseconds, no build. Parses the two headers rather than the asserts TU, so a code that never gets registered there is still caught, and diffs our snapshot against the live c64-wireguard/src/net_abi.inc when a checkout is present. Absent one it skips with a reason; pytest.ini already sets addopts = -ra, so that reason prints on every run. $8A UCI_ERR_LONG_READ is the one deliberate overlap -- their allocation, mirrored here reserved-never-emitted -- so it is asserted to EQUAL their value rather than differ from it, and is excluded from the collision sweep by name, never by value. Cross-repo state at c64-wireguard cf7b41e: no collision. All 16 codes we define are listed in their registry with the ownership we claim. Co-Authored-By: Claude Opus 5 (1M context) --- CLAUDE.md | 21 ++- pytest.ini | 1 + src/net/ip65/ip65_errors.inc | 10 + src/net/uci/uci_errors.inc | 18 ++ src/net_err_registry_asserts.s | 143 +++++++++++++++ tools/test_net_err_registry.py | 322 +++++++++++++++++++++++++++++++++ 6 files changed, 512 insertions(+), 3 deletions(-) create mode 100644 src/net_err_registry_asserts.s create mode 100644 tools/test_net_err_registry.py diff --git a/CLAUDE.md b/CLAUDE.md index 080a3f3..c56107b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -271,9 +271,9 @@ Switching backend = a different cfg + different `src/net//*.o`. SPEC §13, issue #70). **§13 was retired at contract v1.0.0; every §13.x number in this section resolves at tag `v0.17.1`, nowhere else.** No §13 assert has a contract-derived counterparty, so no contract release can -break a build — but the error codes below are asserted NOWHERE, and that -is the live hazard (see the allocation note). `src/net_abi.inc` is the -normative source now. `boot.s`, `http.s`, `tls_record_io.s` and `tls13.s` +break a build. The error codes below used to be asserted NOWHERE; they +are asserted now (#184) — see the allocation note. `src/net_abi.inc` is +the normative source now. `boot.s`, `http.s`, `tls_record_io.s` and `tls13.s` `.include` it and import no `net_*` symbol directly, so a backend that drops a symbol fails the link by name on both backends. Surface: @@ -304,6 +304,21 @@ drops a symbol fails the link by name on both backends. Surface: new code in `c64-wireguard/src/net_abi.inc`, which declares itself canonical for both ranges, then here. It owns `$8C-$8F` and `$46-$49`, which our two error headers used to present as free (#184). + **That is mechanically enforced now, in two halves (#184).** + `src/net_err_registry_asserts.s` is `$(wildcard src/*.s)`, so it + assembles into every build on both backends and emits **no bytes**: it + holds the peer's codes as `NET_ERR_PEER_*` equates and `.assert`s (scope + `error`, so ca65 settles it before ld65) that no code of ours lands on + one, that each is in family range, and that no published value has been + reassigned. Its blind spot — a code never registered in it — is closed + by `tools/test_net_err_registry.py`, which parses the two headers + instead, and which also diffs our snapshot against the live peer file + when a c64-wireguard checkout is present (`C64_WIREGUARD_ROOT`, else + `../c64-wireguard`, else `~/Documents/c64-wireguard`). Without one it + SKIPS, loudly — `pytest.ini` sets `addopts = -ra` so the reason prints + on every run. Adding a code takes all three edits and the suite is red + until they agree. The `NET_FAMILY_*` bits in `src/net/net_families.inc` + are the same cross-repo copy problem and are still unguarded. - Gone, per §13.1: `net_tcp_set_recv_cb` (stub), `net_recv_ready`, `net_dhcp` (alias), and `net_print_ip` — IP printing is consumer UI and is now `print_local_ip` in `boot.s`, one copy for both backends. diff --git a/pytest.ini b/pytest.ini index bdfdd44..4edb672 100644 --- a/pytest.ini +++ b/pytest.ini @@ -49,6 +49,7 @@ testpaths = tools/test_device_lock_timeout.py tools/test_flags_stamp_skip_is_loud.py tools/test_ip65_hw_checks_unit.py + tools/test_net_err_registry.py tools/test_net_test_env.py tools/test_package_verify.py tools/test_pins.py diff --git a/src/net/ip65/ip65_errors.inc b/src/net/ip65/ip65_errors.inc index dce36e2..a83bd50 100644 --- a/src/net/ip65/ip65_errors.inc +++ b/src/net/ip65/ip65_errors.inc @@ -30,6 +30,16 @@ ; collision, caught in review three weeks after the $88 one. Remapped before ; merge. Comments only, deliberately: an equate here would imply we emit ; them. Their registry is the authority, not this snapshot (#184). +; +; THAT SNAPSHOT IS NOW MACHINE-CHECKED (#184), because prose does not fail a +; build. src/net_err_registry_asserts.s assembles into every build on both +; backends, emits no bytes, and `.assert`s that no code defined here lands +; on a peer-owned value, that each is in the $40-$7F family range, and that +; no published value has been reassigned. tools/test_net_err_registry.py +; parses THIS file, so a code that never gets registered in that TU is +; caught too, and diffs the snapshot against the live peer registry when a +; c64-wireguard checkout is present. Adding a code means all three: their +; registry, the equate here, the registration there. .ifndef IP65_ERRORS_INC_INCLUDED IP65_ERRORS_INC_INCLUDED = 1 diff --git a/src/net/uci/uci_errors.inc b/src/net/uci/uci_errors.inc index be9cf4f..a6e5530 100644 --- a/src/net/uci/uci_errors.inc +++ b/src/net/uci/uci_errors.inc @@ -34,6 +34,24 @@ ; them. Re-read their registry before allocating; this list is a snapshot, ; not the authority (#184). ; +; THAT SNAPSHOT IS NOW MACHINE-CHECKED (#184). The list above is prose, and +; prose does not fail a build, so two guards stand behind it: +; +; src/net_err_registry_asserts.s assembles into EVERY build, both +; backends, every profile, and emits no bytes. It carries the peer's +; codes as NET_ERR_PEER_* equates and `.assert`s that no code defined +; here lands on one, that each is in family range, and that no +; published value has been reassigned. Minting $8C fails ca65 with a +; named message instead of linking clean. +; tools/test_net_err_registry.py parses THIS file rather than the +; asserts TU, so a new code that never gets registered there is still +; caught, and diffs our snapshot against the live +; c64-wireguard/src/net_abi.inc when a checkout is present. +; +; Adding a code: allocate in c64-wireguard/src/net_abi.inc, add the equate +; here, then register it in src/net_err_registry_asserts.s. The suite fails +; until all three agree. +; ; One inconsistency, called out rather than hidden: $8A below IS a real ; equate, and is likewise reserved-never-emitted. It predates this block and ; stays — a published value is never reassigned, and removing the equate diff --git a/src/net_err_registry_asserts.s b/src/net_err_registry_asserts.s new file mode 100644 index 0000000..cad4f35 --- /dev/null +++ b/src/net_err_registry_asserts.s @@ -0,0 +1,143 @@ +; src/net_err_registry_asserts.s — mechanical guard over the fleet's shared +; `net_last_error` number space (issue #184). +; +; WHAT THIS IS FOR. The ip65 family ($40-$7F) and the UCI family ($80-$BF) +; are ONE namespace each, shared by every adapter in the fleet — c64-https +; and c64-wireguard today. c64-lib-contract SPEC §13.2 used to hold the +; cross-repo allocation table; it was retired wholesale at contract v1.0.0 +; and the registry moved to `c64-wireguard/src/net_abi.inc`, which declares +; itself canonical for BOTH ranges. That file is the authority. This one is +; a machine-checked snapshot of it. +; +; Until this TU existed, the shared codes were asserted NOWHERE: our two +; headers listed the peer's allocations in prose only, so minting over one +; produced a clean build and a byte that meant two different things in two +; products. That has already happened twice in the fleet — $88 (four days +; live) and wg#120's first commit, which minted $40-$44 over our $41-$45 and +; was caught only by a human reviewer. Prose caught the second one. Nothing +; caught the first. +; +; WHAT IT COSTS. Nothing. Every symbol below is an assemble-time equate and +; every check is a `.assert`; this TU emits no bytes and claims no segment. +; It is picked up by the Makefile's `$(wildcard src/*.s)`, so it assembles +; into EVERY build, both backends, every profile — an ip65 build checks the +; UCI codes and vice versa, which is the point: the collision this guards +; against is cross-product, not cross-profile. +; +; WHY EQUATES HERE WHEN THE HEADERS DELIBERATELY USE COMMENTS. #185 kept the +; peer's codes as comments in src/net/{uci/uci_errors.inc,ip65/ip65_errors.inc} +; because an equate in OUR error namespace would read as "we emit this". The +; `NET_ERR_PEER_*` names below are a separate, obviously-foreign namespace +; whose only consumer is the assertions in this file; nothing emits them and +; nothing may. This is the shape c64-wireguard already uses for their own +; reserved $47 (`.export` + asserts, src/net/ip65/net.s), which +; uci_errors.inc names as "the more durable shape" to copy. +; +; SCOPE, honestly. This file cannot see a code that is not written into it, +; and it cannot see the peer repository. Two limits, one covered each: +; - a new equate added to our headers but not registered here is caught by +; tools/test_net_err_registry.py, which parses the headers themselves; +; - drift against the peer's live registry is caught by the same suite when +; a c64-wireguard checkout is present (C64_WIREGUARD_ROOT, or the +; sibling default). Absent one, it says so rather than passing quietly. +; +; MAINTENANCE. Adding a code: allocate it in c64-wireguard/src/net_abi.inc +; FIRST, then in the emitting header here, then register it below. Never +; reassign a published value — the whole registry rests on that one rule. + +.include "uci/uci_errors.inc" ; -I src/net; UCI_ERR_* ($80-$BF) +.include "ip65/ip65_errors.inc" ; -I src/net; NET_ERR_IP65_* ($40-$7F) + +; --- Family range bounds (retired SPEC §13.2, now the peer registry) ------- +NET_ERR_IP65_FAMILY_LO = $40 +NET_ERR_IP65_FAMILY_HI = $7F +NET_ERR_UCI_FAMILY_LO = $80 +NET_ERR_UCI_FAMILY_HI = $BF + +; --- Codes owned by c64-wireguard. We emit NONE of these. ----------------- +; Snapshot of c64-wireguard/src/net_abi.inc @ cf7b41e (2026-09-07). +NET_ERR_PEER_IP65_UDP_LISTEN = $46 +NET_ERR_PEER_IP65_UDP_SEND = $47 ; reserved there, never emitted +NET_ERR_PEER_IP65_WAIT_TIMEOUT = $48 +NET_ERR_PEER_IP65_UDP_UNBIND = $49 +NET_ERR_PEER_UCI_LONG_READ = $8A ; we mirror this one — see below +NET_ERR_PEER_UCI_SEND_TOO_LONG = $8C +NET_ERR_PEER_UCI_OPEN_REFUSED = $8D +NET_ERR_PEER_UCI_CMD_UNKNOWN = $8E +NET_ERR_PEER_UCI_SHORT_READ = $8F + +; --- Assertions ----------------------------------------------------------- +; `error` scope, not `lderror`: every operand is a local assemble-time +; equate, so ca65 settles these before ld65 is reached and a collision fails +; the build at the offending object rather than at the link. + +.macro NET_ERR_ASSERT_IP65 val, name + .assert (val) >= NET_ERR_IP65_FAMILY_LO && (val) <= NET_ERR_IP65_FAMILY_HI, error, .concat(name, ": outside the ip65 family range $40-$7F (c64-wireguard/src/net_abi.inc registry, #184)") + .assert (val) <> NET_ERR_PEER_IP65_UDP_LISTEN, error, .concat(name, ": collides with c64-wireguard's $46 NET_ERR_IP65_UDP_LISTEN - allocate in c64-wireguard/src/net_abi.inc first (#184)") + .assert (val) <> NET_ERR_PEER_IP65_UDP_SEND, error, .concat(name, ": collides with c64-wireguard's $47 NET_ERR_IP65_UDP_SEND (reserved, never emitted) - allocate in c64-wireguard/src/net_abi.inc first (#184)") + .assert (val) <> NET_ERR_PEER_IP65_WAIT_TIMEOUT, error, .concat(name, ": collides with c64-wireguard's $48 NET_ERR_IP65_WAIT_TIMEOUT - allocate in c64-wireguard/src/net_abi.inc first (#184)") + .assert (val) <> NET_ERR_PEER_IP65_UDP_UNBIND, error, .concat(name, ": collides with c64-wireguard's $49 NET_ERR_IP65_UDP_UNBIND - allocate in c64-wireguard/src/net_abi.inc first (#184)") +.endmacro + +.macro NET_ERR_ASSERT_UCI val, name + .assert (val) >= NET_ERR_UCI_FAMILY_LO && (val) <= NET_ERR_UCI_FAMILY_HI, error, .concat(name, ": outside the UCI family range $80-$BF (c64-wireguard/src/net_abi.inc registry, #184)") + .assert (val) <> NET_ERR_PEER_UCI_LONG_READ, error, .concat(name, ": collides with c64-wireguard's $8A UCI_ERR_LONG_READ - allocate in c64-wireguard/src/net_abi.inc first (#184)") + .assert (val) <> NET_ERR_PEER_UCI_SEND_TOO_LONG, error, .concat(name, ": collides with c64-wireguard's $8C UCI_ERR_SEND_TOO_LONG - allocate in c64-wireguard/src/net_abi.inc first (#184)") + .assert (val) <> NET_ERR_PEER_UCI_OPEN_REFUSED, error, .concat(name, ": collides with c64-wireguard's $8D UCI_ERR_OPEN_REFUSED - allocate in c64-wireguard/src/net_abi.inc first (#184)") + .assert (val) <> NET_ERR_PEER_UCI_CMD_UNKNOWN, error, .concat(name, ": collides with c64-wireguard's $8E UCI_ERR_CMD_UNKNOWN - allocate in c64-wireguard/src/net_abi.inc first (#184)") + .assert (val) <> NET_ERR_PEER_UCI_SHORT_READ, error, .concat(name, ": collides with c64-wireguard's $8F UCI_ERR_SHORT_READ - allocate in c64-wireguard/src/net_abi.inc first (#184)") +.endmacro + +; ip65 family — every code src/net/ip65/ip65_errors.inc defines. +NET_ERR_ASSERT_IP65 NET_ERR_IP65_INIT, "NET_ERR_IP65_INIT" +NET_ERR_ASSERT_IP65 NET_ERR_IP65_DHCP, "NET_ERR_IP65_DHCP" +NET_ERR_ASSERT_IP65 NET_ERR_IP65_DNS, "NET_ERR_IP65_DNS" +NET_ERR_ASSERT_IP65 NET_ERR_IP65_CONNECT, "NET_ERR_IP65_CONNECT" +NET_ERR_ASSERT_IP65 NET_ERR_IP65_SEND, "NET_ERR_IP65_SEND" + +; UCI family — every code src/net/uci/uci_errors.inc defines EXCEPT +; UCI_ERR_LONG_READ, handled immediately below. +NET_ERR_ASSERT_UCI UCI_ERR_NOT_PRESENT, "UCI_ERR_NOT_PRESENT" +NET_ERR_ASSERT_UCI UCI_ERR_CMD_FAILED, "UCI_ERR_CMD_FAILED" +NET_ERR_ASSERT_UCI UCI_ERR_NO_IP, "UCI_ERR_NO_IP" +NET_ERR_ASSERT_UCI UCI_ERR_CONNECT_FAIL, "UCI_ERR_CONNECT_FAIL" +NET_ERR_ASSERT_UCI UCI_ERR_SEND_FAIL, "UCI_ERR_SEND_FAIL" +NET_ERR_ASSERT_UCI UCI_ERR_READ_FAIL, "UCI_ERR_READ_FAIL" +NET_ERR_ASSERT_UCI UCI_ERR_SHORT_WRITE, "UCI_ERR_SHORT_WRITE" +NET_ERR_ASSERT_UCI UCI_ERR_NO_SOCKET, "UCI_ERR_NO_SOCKET" +NET_ERR_ASSERT_UCI UCI_ERR_WAIT_TIMEOUT, "UCI_ERR_WAIT_TIMEOUT" +NET_ERR_ASSERT_UCI UCI_ERR_BAD_READ_HDR, "UCI_ERR_BAD_READ_HDR" + +; THE ONE DELIBERATE OVERLAP. UCI_ERR_LONG_READ = $8A is c64-wireguard's +; allocation, mirrored here as a reserved-never-emitted equate so the name +; is readable in our diagnostics (see the block in uci_errors.inc). It is +; therefore the one code that must EQUAL a peer value instead of differing +; from one — checked in that direction so the mirror cannot silently drift +; off theirs, and named so nobody mistakes it for a missed collision. +.assert UCI_ERR_LONG_READ = NET_ERR_PEER_UCI_LONG_READ, error, "UCI_ERR_LONG_READ must mirror c64-wireguard's $8A exactly; it is their allocation, reserved and never emitted here (#184)" + +; PUBLISHED VALUES, PINNED. The registry's single rule is that a published +; value is never reassigned — not renumbered to close a gap, not reused +; because a code turned out unreachable. These literals are that rule made +; mechanical, and they also give the set pairwise distinctness for free. +; Changing one of these numbers is not a refactor; it is a fleet-wide +; incompatibility, and it must fail here. +.assert NET_ERR_IP65_INIT = $41, error, "NET_ERR_IP65_INIT is published as $41 and must never be reassigned (#184)" +.assert NET_ERR_IP65_DHCP = $42, error, "NET_ERR_IP65_DHCP is published as $42 and must never be reassigned (#184)" +.assert NET_ERR_IP65_DNS = $43, error, "NET_ERR_IP65_DNS is published as $43 and must never be reassigned (#184)" +.assert NET_ERR_IP65_CONNECT = $44, error, "NET_ERR_IP65_CONNECT is published as $44 and must never be reassigned (#184)" +.assert NET_ERR_IP65_SEND = $45, error, "NET_ERR_IP65_SEND is published as $45 and must never be reassigned (#184)" +.assert UCI_ERR_NOT_PRESENT = $81, error, "UCI_ERR_NOT_PRESENT is published as $81 and must never be reassigned (#184)" +.assert UCI_ERR_CMD_FAILED = $82, error, "UCI_ERR_CMD_FAILED is published as $82 and must never be reassigned (#184)" +.assert UCI_ERR_NO_IP = $83, error, "UCI_ERR_NO_IP is published as $83 and must never be reassigned (#184)" +.assert UCI_ERR_CONNECT_FAIL = $84, error, "UCI_ERR_CONNECT_FAIL is published as $84 and must never be reassigned (#184)" +.assert UCI_ERR_SEND_FAIL = $85, error, "UCI_ERR_SEND_FAIL is published as $85 and must never be reassigned (#184)" +.assert UCI_ERR_READ_FAIL = $86, error, "UCI_ERR_READ_FAIL is published as $86 and must never be reassigned (#184)" +.assert UCI_ERR_SHORT_WRITE = $87, error, "UCI_ERR_SHORT_WRITE is published as $87 and must never be reassigned (#184)" +.assert UCI_ERR_NO_SOCKET = $88, error, "UCI_ERR_NO_SOCKET is published as $88 and must never be reassigned (#184)" +.assert UCI_ERR_WAIT_TIMEOUT = $89, error, "UCI_ERR_WAIT_TIMEOUT is published as $89 and must never be reassigned (#184)" +.assert UCI_ERR_LONG_READ = $8A, error, "UCI_ERR_LONG_READ is published as $8A and must never be reassigned (#184)" +.assert UCI_ERR_BAD_READ_HDR = $8B, error, "UCI_ERR_BAD_READ_HDR is published as $8B and must never be reassigned (#184)" + +; $00 is "no error" in every family, fleet-wide, and is not allocatable. +.assert UCI_ERR_OK = $00, error, "UCI_ERR_OK must stay $00 - 'no error' is fleet-wide, not a UCI allocation (#184)" diff --git a/tools/test_net_err_registry.py b/tools/test_net_err_registry.py new file mode 100644 index 0000000..7a8f56e --- /dev/null +++ b/tools/test_net_err_registry.py @@ -0,0 +1,322 @@ +#!/usr/bin/env python3 +"""Guard the fleet's shared `net_last_error` number space (issue #184). + +Pure logic: reads three source files off disk, parses them, compares. No +build, no VICE, no hardware, milliseconds. Runs under pytest (it is listed +in ``pytest.ini``'s ``testpaths``) and standalone:: + + python3 tools/test_net_err_registry.py + +WHY THIS EXISTS. The ip65 family ($40-$7F) and the UCI family ($80-$BF) are +one namespace each, shared by c64-https and c64-wireguard. c64-lib-contract +SPEC §13.2 used to hold the cross-repo allocation table; §13 was retired +wholesale at contract v1.0.0 and the registry moved to +``c64-wireguard/src/net_abi.inc``, which declares itself canonical for both +ranges. Two collisions have already happened in this fleet — $88, live for +four days, and wg#120's first commit minting $40-$44 over our $41-$45, +caught by a human reviewer. Until #184 nothing mechanical checked either +range in this repo. + +WHAT IS CHECKED, AND WHERE THE OTHER HALF LIVES. +``src/net_err_registry_asserts.s`` is the link-time half: it fails the ca65 +assemble of every profile, both backends, if a code it knows about lands on +a peer-owned value or a published value is reassigned. It costs no bytes. +Its blind spot is a code that never gets registered in it, and it cannot see +the peer repository at all. This suite covers exactly those two gaps: + + 1-3. Structural, always run. Every error code defined in our two headers + is in its family range, is registered in the asserts TU, and does not + sit on a peer-owned value. (3) is the assembler's check repeated from + the headers' side, which is the point: (2)+(3) together mean a new + code cannot dodge the assembler by simply not being registered. + 4. Snapshot integrity: the NET_ERR_PEER_* table in the asserts TU agrees + with the prose lists in the two headers. + 5-7. Cross-repo drift, against the live peer registry. SKIPPED when no + c64-wireguard checkout is found — pytest.ini sets ``addopts = -ra``, + so the skip and its reason are printed on every run rather than + vanishing into a green count. Point it at a checkout with + ``C64_WIREGUARD_ROOT=/path/to/c64-wireguard``; the sibling default + is ``../c64-wireguard`` relative to this repo, then + ``~/Documents/c64-wireguard``. + +WHAT IT DELIBERATELY DOES NOT DO. It never edits, and never asserts +anything about, the peer repository's own correctness. A code of ours that +their registry has claimed is reported as a finding for a human to take +cross-repo; this suite's job is to make it impossible to not notice. +""" + +import os +import re +import sys +from pathlib import Path + +REPO = Path(__file__).resolve().parent.parent +UCI_HEADER = REPO / "src" / "net" / "uci" / "uci_errors.inc" +IP65_HEADER = REPO / "src" / "net" / "ip65" / "ip65_errors.inc" +ASSERTS_TU = REPO / "src" / "net_err_registry_asserts.s" + +IP65_FAMILY = (0x40, 0x7F) +UCI_FAMILY = (0x80, 0xBF) + +# The one code we define that is a c64-wireguard allocation: mirrored here, +# reserved, never emitted, so the name is readable in our diagnostics. It is +# checked in the opposite direction from every other code (it must EQUAL +# theirs), and is excluded from the collision sweep by name, never by value. +MIRRORED = "UCI_ERR_LONG_READ" + +# `NAME = $hh` where hh lands in either family range. Two hex digits only: +# a four-digit $DFxx is a register, not an error code, and the sub-$40 +# values in these files ($00 = OK) are not allocatable. +_EQUATE_RE = re.compile(r"^\s*([A-Za-z_][A-Za-z0-9_]*)\s*=\s*\$([0-9A-Fa-f]{2})\s*(?:;.*)?$") + +# `NET_ERR_PEER_NAME = $hh` in the asserts TU. +_PEER_RE = re.compile(r"^\s*(NET_ERR_PEER_[A-Za-z0-9_]+)\s*=\s*\$([0-9A-Fa-f]{2})\b") + +# A registry row in c64-wireguard/src/net_abi.inc: +# ; $8E UCI_ERR_CMD_UNKNOWN OURS, minted here (PR #112) +_PEER_ROW_RE = re.compile(r"^;\s+\$([0-9A-Fa-f]{2})\s+([A-Z][A-Za-z0-9_]*)\s{2,}(.+?)\s*$") + +# A comment row in one of our headers listing a peer allocation: +# ; $8C UCI_ERR_SEND_TOO_LONG theirs; ... +_OUR_COMMENT_ROW_RE = re.compile(r"^;\s+\$([0-9A-Fa-f]{2})\s+([A-Z][A-Za-z0-9_]*)\s+\S") + + +def _in(value, family): + return family[0] <= value <= family[1] + + +def _read(path): + return path.read_text(encoding="utf-8").splitlines() + + +def _our_codes(): + """{name: value} for every error code our two headers define.""" + codes = {} + for path, family in ((UCI_HEADER, UCI_FAMILY), (IP65_HEADER, IP65_FAMILY)): + for line in _read(path): + m = _EQUATE_RE.match(line) + if not m: + continue + name, value = m.group(1), int(m.group(2), 16) + if not (_in(value, IP65_FAMILY) or _in(value, UCI_FAMILY)): + continue + assert name not in codes, f"{name} defined twice across the headers" + codes[name] = (value, path, family) + return codes + + +def _peer_snapshot(): + """{name: value} for the NET_ERR_PEER_* table in the asserts TU.""" + return {m.group(1): int(m.group(2), 16) + for m in (_PEER_RE.match(l) for l in _read(ASSERTS_TU)) if m} + + +def _registered_names(): + """Names the asserts TU actually puts through a collision macro.""" + names = set() + for line in _read(ASSERTS_TU): + m = re.match(r"^NET_ERR_ASSERT_(?:IP65|UCI)\s+([A-Za-z0-9_]+)\s*,", line) + if m: + names.add(m.group(1)) + return names + + +def _wireguard_root(): + env = os.environ.get("C64_WIREGUARD_ROOT") + candidates = [Path(env)] if env else [REPO.parent / "c64-wireguard", + Path.home() / "Documents" / "c64-wireguard"] + for c in candidates: + if (c / "src" / "net_abi.inc").is_file(): + return c + return None + + +def _peer_registry(root): + """{value: (name, owner)} parsed from the peer's canonical table. + + owner is "c64-wireguard", "c64-https", or "other" (contract-generic + rows such as $01). Only rows inside a family range are returned. + """ + rows = {} + for line in _read(root / "src" / "net_abi.inc"): + m = _PEER_ROW_RE.match(line) + if not m: + continue + value = int(m.group(1), 16) + if not (_in(value, IP65_FAMILY) or _in(value, UCI_FAMILY)): + continue + origin = m.group(3) + if "c64-https" in origin: + owner = "c64-https" + elif re.search(r"\bours\b", origin, re.IGNORECASE): + owner = "c64-wireguard" + else: + owner = "other" + rows[value] = (m.group(2), owner) + return rows + + +# -------------------------------------------------------------------------- +# 1-4: structural, no peer checkout needed. +# -------------------------------------------------------------------------- + +def test_every_code_is_in_its_family_range(): + for name, (value, path, family) in sorted(_our_codes().items()): + assert _in(value, family), ( + f"{name} = ${value:02X} in {path.name} is outside its family range " + f"${family[0]:02X}-${family[1]:02X} (#184)") + + +def test_every_code_is_registered_in_the_asserts_tu(): + """The assembler cannot check a code nobody registered with it.""" + registered = _registered_names() | {MIRRORED} + missing = sorted(n for n in _our_codes() if n not in registered) + assert not missing, ( + f"error codes defined in our headers but not registered in " + f"{ASSERTS_TU.relative_to(REPO)}: {missing}. Add a " + f"NET_ERR_ASSERT_IP65/NET_ERR_ASSERT_UCI line for each, or the " + f"link-time collision guard silently does not cover them (#184).") + + +def test_no_code_of_ours_sits_on_a_peer_owned_value(): + peer = _peer_snapshot() + by_value = {} + for pname, pvalue in peer.items(): + by_value.setdefault(pvalue, []).append(pname) + clashes = [] + for name, (value, _path, _family) in sorted(_our_codes().items()): + if name == MIRRORED: + continue + if value in by_value: + clashes.append(f"{name} = ${value:02X} == {by_value[value]}") + assert not clashes, ( + "codes of ours land on c64-wireguard-owned values: " + + "; ".join(clashes) + + ". Allocate in c64-wireguard/src/net_abi.inc first (#184).") + + +def test_the_mirrored_code_tracks_the_peer_value(): + codes = _our_codes() + peer = _peer_snapshot() + assert MIRRORED in codes, f"{MIRRORED} vanished from uci_errors.inc" + assert peer.get("NET_ERR_PEER_UCI_LONG_READ") == codes[MIRRORED][0], ( + f"{MIRRORED} must mirror c64-wireguard's $8A exactly; it is their " + f"allocation, reserved and never emitted here (#184).") + + +def test_header_prose_lists_match_the_snapshot_table(): + """The comment lists #185 put in the headers must not drift from the + NET_ERR_PEER_* equates the assembler actually checks.""" + snapshot = set(_peer_snapshot().values()) + ours = {v for v, _p, _f in _our_codes().values()} + listed = set() + for path in (UCI_HEADER, IP65_HEADER): + for line in _read(path): + m = _OUR_COMMENT_ROW_RE.match(line) + if m: + value = int(m.group(1), 16) + if (_in(value, IP65_FAMILY) or _in(value, UCI_FAMILY)) \ + and value not in ours: + listed.add(value) + only_prose = sorted(listed - snapshot) + only_table = sorted(snapshot - listed - ours) + assert not only_prose and not only_table, ( + "the peer-owned codes named in the headers' comments and the " + "NET_ERR_PEER_* table in net_err_registry_asserts.s disagree: " + f"only in prose {[f'${v:02X}' for v in only_prose]}, " + f"only in the table {[f'${v:02X}' for v in only_table]} (#184).") + + +# -------------------------------------------------------------------------- +# 5-7: cross-repo drift. Needs a c64-wireguard checkout. +# -------------------------------------------------------------------------- + +def _require_peer(): + root = _wireguard_root() + if root is None: + try: + import pytest + except ImportError: + return None + pytest.skip( + "no c64-wireguard checkout found, so the snapshot in " + "src/net_err_registry_asserts.s is UNVERIFIED against the " + "canonical registry. Set C64_WIREGUARD_ROOT=/path/to/c64-wireguard " + "(or place it at ../c64-wireguard) to cover this (#184).") + return root + + +def test_snapshot_matches_the_peer_registry(): + root = _require_peer() + if root is None: + print("SKIP: no c64-wireguard checkout") + return + registry = _peer_registry(root) + theirs = {v for v, (_n, owner) in registry.items() if owner == "c64-wireguard"} + snapshot = set(_peer_snapshot().values()) + missing = sorted(theirs - snapshot) + stale = sorted(snapshot - theirs) + assert not missing and not stale, ( + f"our NET_ERR_PEER_* snapshot has drifted from {root}/src/net_abi.inc: " + f"they own but we do not list {[f'${v:02X}' for v in missing]}; " + f"we list but they no longer own {[f'${v:02X}' for v in stale]}. " + f"Update src/net_err_registry_asserts.s and both headers (#184).") + + +def test_no_code_of_ours_is_claimed_by_the_peer_registry(): + """The finding this whole ticket exists to make impossible to miss.""" + root = _require_peer() + if root is None: + print("SKIP: no c64-wireguard checkout") + return + registry = _peer_registry(root) + bad = [] + for name, (value, _path, _family) in sorted(_our_codes().items()): + if name == MIRRORED: + continue + row = registry.get(value) + if row and row[1] == "c64-wireguard": + bad.append(f"${value:02X} is ours as {name} and theirs as {row[0]}") + assert not bad, ( + "LIVE CROSS-REPO COLLISION — " + "; ".join(bad) + + f". Registry: {root}/src/net_abi.inc. Do not renumber unilaterally; " + "a published value is never reassigned. Take this to the " + "c64-wireguard lane (#184).") + + +def test_every_code_of_ours_appears_in_the_peer_registry(): + root = _require_peer() + if root is None: + print("SKIP: no c64-wireguard checkout") + return + registry = _peer_registry(root) + unlisted = sorted( + f"{name} = ${value:02X}" + for name, (value, _p, _f) in _our_codes().items() + if value not in registry) + assert not unlisted, ( + f"codes we define that the canonical registry does not list: " + f"{unlisted}. Allocate them in {root}/src/net_abi.inc — a code that " + f"is not in the registry is a code the next lane will mint over " + f"(#184).") + + +def main(): + failures = 0 + for name, fn in sorted(globals().items()): + if not name.startswith("test_") or not callable(fn): + continue + try: + fn() + print(f"PASS {name}") + except AssertionError as exc: + failures += 1 + print(f"FAIL {name}\n {exc}") + root = _wireguard_root() + print(f"\npeer registry: {root or 'NOT FOUND (cross-repo checks skipped)'}") + print(f"{'FAILED' if failures else 'OK'} — {failures} failure(s)") + return 1 if failures else 0 + + +if __name__ == "__main__": + sys.exit(main()) From 53a35bb54fc17cfacd452372a3f64593fabbea2a Mon Sep 17 00:00:00 2001 From: JC-000 <3798556+JC-000@users.noreply.github.com> Date: Mon, 7 Sep 2026 23:19:35 -0500 Subject: [PATCH 2/4] fix(net): close the registry guard's crash, scope its claims (#184 review) Adversary review of the first commit. The zero-byte / every-profile / cross-product engineering held; these are the guard's advertised scope and one crash. F3 -- the standalone entry point died mid-suite. _require_peer() called pytest.skip(), which raises Skipped, a BaseException; main() caught only AssertionError. Five of eight checks never ran, no summary printed, and the exit 1 meant "a check FAILED" under this repo's contract. That is verbatim the failure _skip_policy.VoluntarySkip exists to prevent, and the `if root is None` fallbacks were dead code whenever pytest was importable. F4 -- and it re-decided the skip policy locally instead of using tools/_skip_policy.py, so it carried no coverage clause and no opt_out_env. Now require(..., executed=6, total=10, certifies=..., opt_out_env= "C64_ALLOW_SKIP"): a missing peer checkout is an INVOLUNTARY skip, so the four cross-repo checks FAIL rather than pass quietly. C64_ALLOW_SKIP=1 is the loud opt-out. The six structural checks run either way. F2 -- nothing checked our own set for pairwise distinctness. UCI_ERR_DUP = $88, duly registered, passed all eight checks including the peer one, because $88 IS in their registry under another name. The assembler cannot express this (its literal pins only cover codes that existed when they were written), so it is a new suite check. F1 -- the parser knew one spelling. Widened to `NAME = ddd` and `.define NAME $hh`; verified no existing decimal equate in either header falls in $40-$BF. Expression-valued equates are DECLARED OUT OF SCOPE in all four documentation sites: evaluating them needs an assembler, and a parser that silently mis-evaluates one would be worse than one that visibly does not try. F5 -- the $8A mirror compared values only, so a peer RENAME left every check green while our diagnostics carried a dead name. New check compares names too, for the whole snapshot; the commit message's "cannot silently drift off theirs" was true of the value and false of the name, and the TU comment now says which. F6 -- the macros' peer list is hand-written and ca65 cannot iterate a table, so a snapshot row with no matching .assert was silently unchecked. Now checked by the suite, and noted where someone editing the table sees it. F7 -- "adding a code takes all three edits and the suite is red until they agree" claimed a completeness F1 disproves. Corrected in both headers, the TU header and CLAUDE.md, each of which now carries what the pair does NOT cover. PRG sha256 unchanged on all five profiles; no assertion logic changed. Co-Authored-By: Claude Opus 5 (1M context) --- CLAUDE.md | 21 ++- src/net/ip65/ip65_errors.inc | 7 + src/net/uci/uci_errors.inc | 16 +- src/net_err_registry_asserts.s | 48 ++++-- tools/test_net_err_registry.py | 258 +++++++++++++++++++++++++-------- 5 files changed, 268 insertions(+), 82 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index c56107b..db0ef26 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -312,13 +312,20 @@ drops a symbol fails the link by name on both backends. Surface: one, that each is in family range, and that no published value has been reassigned. Its blind spot — a code never registered in it — is closed by `tools/test_net_err_registry.py`, which parses the two headers - instead, and which also diffs our snapshot against the live peer file - when a c64-wireguard checkout is present (`C64_WIREGUARD_ROOT`, else - `../c64-wireguard`, else `~/Documents/c64-wireguard`). Without one it - SKIPS, loudly — `pytest.ini` sets `addopts = -ra` so the reason prints - on every run. Adding a code takes all three edits and the suite is red - until they agree. The `NET_FAMILY_*` bits in `src/net/net_families.inc` - are the same cross-repo copy problem and are still unguarded. + instead; it also catches two of our own names on one byte (which the + assembler cannot express) and diffs our snapshot's **values and names** + against the live peer file when a c64-wireguard checkout is present + (`C64_WIREGUARD_ROOT`, else `../c64-wireguard`, else + `~/Documents/c64-wireguard`). A missing checkout is an **involuntary + skip**, so those four checks FAIL rather than pass quietly + (`tools/_skip_policy.py`, #178); `C64_ALLOW_SKIP=1` is the loud opt-out + and still prints the vacuity block. **Scope the guarantee correctly: + both halves are text-level, and recognise only `NAME = $hh`, + `NAME = ddd` and `.define NAME $hh`** — an expression-valued equate + (`UCI_ERR_NEW = UCI_ERR_NO_SOCKET + 4`) passes both while colliding, and + is documented out of scope rather than half-handled. Write literals. The + `NET_FAMILY_*` bits in `src/net/net_families.inc` are the same cross-repo + copy problem and are still unguarded. - Gone, per §13.1: `net_tcp_set_recv_cb` (stub), `net_recv_ready`, `net_dhcp` (alias), and `net_print_ip` — IP printing is consumer UI and is now `print_local_ip` in `boot.s`, one copy for both backends. diff --git a/src/net/ip65/ip65_errors.inc b/src/net/ip65/ip65_errors.inc index a83bd50..36ab4b7 100644 --- a/src/net/ip65/ip65_errors.inc +++ b/src/net/ip65/ip65_errors.inc @@ -40,6 +40,13 @@ ; caught too, and diffs the snapshot against the live peer registry when a ; c64-wireguard checkout is present. Adding a code means all three: their ; registry, the equate here, the registration there. +; +; Narrow, though, and worth knowing before trusting a green run: both +; guards read `NAME = $hh` / `NAME = ddd` / `.define NAME $hh` and nothing +; else, so an EXPRESSION-valued equate passes both while colliding — write +; literals, as every code below does. The cross-repo half needs a peer +; checkout and FAILS without one rather than passing quietly +; (`C64_ALLOW_SKIP=1` opts out loudly). .ifndef IP65_ERRORS_INC_INCLUDED IP65_ERRORS_INC_INCLUDED = 1 diff --git a/src/net/uci/uci_errors.inc b/src/net/uci/uci_errors.inc index a6e5530..6551cce 100644 --- a/src/net/uci/uci_errors.inc +++ b/src/net/uci/uci_errors.inc @@ -49,8 +49,20 @@ ; c64-wireguard/src/net_abi.inc when a checkout is present. ; ; Adding a code: allocate in c64-wireguard/src/net_abi.inc, add the equate -; here, then register it in src/net_err_registry_asserts.s. The suite fails -; until all three agree. +; here, then register it in src/net_err_registry_asserts.s. +; +; WHAT THAT PAIR DOES NOT COVER — the guarantee is real but narrow: +; - **Write the value as a literal.** Both guards recognise `NAME = $hh`, +; `NAME = ddd` and `.define NAME $hh`. An expression — say +; `UCI_ERR_NEW = UCI_ERR_NO_SOCKET + 4` — is invisible to a text +; parser and passes BOTH halves while colliding. Every code below is a +; literal; keep it that way. +; - The cross-repo half needs a c64-wireguard checkout. Without one those +; checks fail (an involuntary skip is a failure) rather than passing; +; `C64_ALLOW_SKIP=1` is the loud opt-out. +; - Nothing here re-verifies c64-wireguard. A collision found against +; their registry is a cross-repo finding for a human, not something +; either repo may renumber unilaterally. ; ; One inconsistency, called out rather than hidden: $8A below IS a real ; equate, and is likewise reserved-never-emitted. It predates this block and diff --git a/src/net_err_registry_asserts.s b/src/net_err_registry_asserts.s index cad4f35..3829bd2 100644 --- a/src/net_err_registry_asserts.s +++ b/src/net_err_registry_asserts.s @@ -33,13 +33,27 @@ ; reserved $47 (`.export` + asserts, src/net/ip65/net.s), which ; uci_errors.inc names as "the more durable shape" to copy. ; -; SCOPE, honestly. This file cannot see a code that is not written into it, -; and it cannot see the peer repository. Two limits, one covered each: -; - a new equate added to our headers but not registered here is caught by -; tools/test_net_err_registry.py, which parses the headers themselves; -; - drift against the peer's live registry is caught by the same suite when -; a c64-wireguard checkout is present (C64_WIREGUARD_ROOT, or the -; sibling default). Absent one, it says so rather than passing quietly. +; SCOPE, honestly — this file is NOT a complete guard on its own, and the +; pair is not complete either. What each half cannot do: +; +; - This file cannot see a code that is not written into it. Registration +; is manual, and a code with no NET_ERR_ASSERT_* line is simply not +; checked here. tools/test_net_err_registry.py covers that by parsing +; the HEADERS instead of this file. +; - This file cannot see two of OUR names on one byte. The literal pins +; below give distinctness only among the codes that existed when they +; were written; a new duplicate passes every macro check, because the +; value is already legitimately ours. Covered by the suite's +; test_our_codes_are_pairwise_distinct. +; - This file cannot see the peer repository. Value and name drift there +; are covered by the suite, and only when a checkout is present — a +; missing one is an involuntary skip, so those checks FAIL rather than +; pass quietly (tools/_skip_policy.py; C64_ALLOW_SKIP=1 to opt out). +; - NEITHER half evaluates ca65. The suite recognises `NAME = $hh`, +; `NAME = ddd` and `.define NAME $hh`; an EXPRESSION-valued equate +; (`UCI_ERR_NEW = UCI_ERR_NO_SOCKET + 4`) is out of scope and passes +; both halves while colliding. Declare codes as literals — every code +; in both headers does. ; ; MAINTENANCE. Adding a code: allocate it in c64-wireguard/src/net_abi.inc ; FIRST, then in the emitting header here, then register it below. Never @@ -56,6 +70,13 @@ NET_ERR_UCI_FAMILY_HI = $BF ; --- Codes owned by c64-wireguard. We emit NONE of these. ----------------- ; Snapshot of c64-wireguard/src/net_abi.inc @ cf7b41e (2026-09-07). +; +; MAINTENANCE: adding a row here is TWO edits. The macros below reference +; these names one `.assert` at a time — they are hand-written, and ca65 +; cannot iterate a table — so a row added here with no matching assert line +; is a peer code the assembler silently does not check. That pairing is +; itself checked, by test_every_snapshot_entry_is_asserted_by_a_macro in +; tools/test_net_err_registry.py; the suite goes red, not the build. NET_ERR_PEER_IP65_UDP_LISTEN = $46 NET_ERR_PEER_IP65_UDP_SEND = $47 ; reserved there, never emitted NET_ERR_PEER_IP65_WAIT_TIMEOUT = $48 @@ -112,8 +133,17 @@ NET_ERR_ASSERT_UCI UCI_ERR_BAD_READ_HDR, "UCI_ERR_BAD_READ_HDR" ; allocation, mirrored here as a reserved-never-emitted equate so the name ; is readable in our diagnostics (see the block in uci_errors.inc). It is ; therefore the one code that must EQUAL a peer value instead of differing -; from one — checked in that direction so the mirror cannot silently drift -; off theirs, and named so nobody mistakes it for a missed collision. +; from one — checked in that direction, and named so nobody mistakes it for +; a missed collision. +; +; SCOPE of that check: it pins the VALUE against this file's snapshot. It +; cannot see a change in the peer repo at all — neither a renumber (which +; the snapshot would have to be updated for anyway) nor a RENAME, which +; moves nothing here and would leave our diagnostics printing a name that +; no longer exists upstream. Both are caught only by +; tools/test_net_err_registry.py, against a live checkout: +; test_snapshot_values_match_the_peer_registry and +; test_snapshot_names_match_the_peer_registry respectively. .assert UCI_ERR_LONG_READ = NET_ERR_PEER_UCI_LONG_READ, error, "UCI_ERR_LONG_READ must mirror c64-wireguard's $8A exactly; it is their allocation, reserved and never emitted here (#184)" ; PUBLISHED VALUES, PINNED. The registry's single rule is that a published diff --git a/tools/test_net_err_registry.py b/tools/test_net_err_registry.py index 7a8f56e..3606b14 100644 --- a/tools/test_net_err_registry.py +++ b/tools/test_net_err_registry.py @@ -17,32 +17,53 @@ caught by a human reviewer. Until #184 nothing mechanical checked either range in this repo. -WHAT IS CHECKED, AND WHERE THE OTHER HALF LIVES. -``src/net_err_registry_asserts.s`` is the link-time half: it fails the ca65 -assemble of every profile, both backends, if a code it knows about lands on -a peer-owned value or a published value is reassigned. It costs no bytes. -Its blind spot is a code that never gets registered in it, and it cannot see -the peer repository at all. This suite covers exactly those two gaps: - - 1-3. Structural, always run. Every error code defined in our two headers - is in its family range, is registered in the asserts TU, and does not - sit on a peer-owned value. (3) is the assembler's check repeated from - the headers' side, which is the point: (2)+(3) together mean a new - code cannot dodge the assembler by simply not being registered. - 4. Snapshot integrity: the NET_ERR_PEER_* table in the asserts TU agrees - with the prose lists in the two headers. - 5-7. Cross-repo drift, against the live peer registry. SKIPPED when no - c64-wireguard checkout is found — pytest.ini sets ``addopts = -ra``, - so the skip and its reason are printed on every run rather than - vanishing into a green count. Point it at a checkout with - ``C64_WIREGUARD_ROOT=/path/to/c64-wireguard``; the sibling default - is ``../c64-wireguard`` relative to this repo, then - ``~/Documents/c64-wireguard``. - -WHAT IT DELIBERATELY DOES NOT DO. It never edits, and never asserts -anything about, the peer repository's own correctness. A code of ours that -their registry has claimed is reported as a finding for a human to take -cross-repo; this suite's job is to make it impossible to not notice. +TWO HALVES. ``src/net_err_registry_asserts.s`` is the assemble-time half: it +fails ca65 on every profile, both backends, if a code it knows about lands +on a peer-owned value or a published value is reassigned, and it costs no +bytes. Its blind spot is a code that is never registered in it, and it +cannot see the peer repository at all. This suite covers exactly those two +gaps, and adds the intra-repo checks the assembler cannot express. + +============================================================================= +WHAT THIS GUARD DOES **NOT** COVER — read before trusting a green run +============================================================================= + +**Only three declaration spellings are recognised**, because this is a text +parser and not ca65: + + NAME = $8C recognised + NAME = 140 recognised + .define NAME $8C recognised + +An **expression-valued** equate is NOT recognised and will pass every check +here while colliding: + + UCI_ERR_NEW = UCI_ERR_NO_SOCKET + 4 NOT COVERED + +Evaluating that needs an assembler, not a regex. It is declared out of +scope rather than half-handled: a parser that silently mis-evaluates one of +these would be worse than one that visibly does not try. If you are adding +a code, write it as a literal — every existing code in both headers does. + +**The assembler half covers only registered codes.** Check 2 below is what +makes that safe, by failing when a code in the headers has no +``NET_ERR_ASSERT_*`` line. The two halves are only jointly complete for the +spellings listed above. + +**Nothing here validates c64-wireguard.** A code of ours their registry has +claimed is reported as a finding for a human to take cross-repo. This suite +never edits, and never asserts the correctness of, the peer repository. + +============================================================================= + +THE CROSS-REPO CHECKS NEED A PEER CHECKOUT, and a missing one is an +INVOLUNTARY skip under ``tools/_skip_policy.py``: the drift checks verify +nothing without it, so they FAIL rather than pass quietly (#158/#165/#178). +Point the suite at a checkout with ``C64_WIREGUARD_ROOT=/path``; the +defaults are ``../c64-wireguard`` then ``~/Documents/c64-wireguard``. A lane +that genuinely has no peer checkout opts out loudly with +``C64_ALLOW_SKIP=1``, which still prints the full vacuity warning. The six +structural checks run either way. """ import os @@ -50,6 +71,9 @@ import sys from pathlib import Path +sys.path.insert(0, str(Path(__file__).resolve().parent)) +from _skip_policy import VoluntarySkip, require # noqa: E402 + REPO = Path(__file__).resolve().parent.parent UCI_HEADER = REPO / "src" / "net" / "uci" / "uci_errors.inc" IP65_HEADER = REPO / "src" / "net" / "ip65" / "ip65_errors.inc" @@ -58,16 +82,23 @@ IP65_FAMILY = (0x40, 0x7F) UCI_FAMILY = (0x80, 0xBF) +TOTAL_CHECKS = 10 +CERTIFIES = ("agreement between this repo's net_last_error allocations and " + "c64-wireguard's canonical registry") + # The one code we define that is a c64-wireguard allocation: mirrored here, # reserved, never emitted, so the name is readable in our diagnostics. It is # checked in the opposite direction from every other code (it must EQUAL # theirs), and is excluded from the collision sweep by name, never by value. MIRRORED = "UCI_ERR_LONG_READ" -# `NAME = $hh` where hh lands in either family range. Two hex digits only: -# a four-digit $DFxx is a register, not an error code, and the sub-$40 -# values in these files ($00 = OK) are not allocatable. -_EQUATE_RE = re.compile(r"^\s*([A-Za-z_][A-Za-z0-9_]*)\s*=\s*\$([0-9A-Fa-f]{2})\s*(?:;.*)?$") +# Declaration spellings we recognise. See the docstring's scope block: an +# expression-valued equate is deliberately out of scope. +# NAME = $hh | NAME = ddd | .define NAME $hh / ddd +_EQUATE_RE = re.compile( + r"^\s*(?:\.define\s+([A-Za-z_][A-Za-z0-9_]*)\s+|" + r"([A-Za-z_][A-Za-z0-9_]*)\s*=\s*)" + r"(?:\$([0-9A-Fa-f]{1,2})|([0-9]{1,3}))\s*(?:;.*)?$") # `NET_ERR_PEER_NAME = $hh` in the asserts TU. _PEER_RE = re.compile(r"^\s*(NET_ERR_PEER_[A-Za-z0-9_]+)\s*=\s*\$([0-9A-Fa-f]{2})\b") @@ -90,14 +121,15 @@ def _read(path): def _our_codes(): - """{name: value} for every error code our two headers define.""" + """{name: (value, path, family)} for every error code our headers define.""" codes = {} for path, family in ((UCI_HEADER, UCI_FAMILY), (IP65_HEADER, IP65_FAMILY)): for line in _read(path): m = _EQUATE_RE.match(line) if not m: continue - name, value = m.group(1), int(m.group(2), 16) + name = m.group(1) or m.group(2) + value = int(m.group(3), 16) if m.group(3) else int(m.group(4), 10) if not (_in(value, IP65_FAMILY) or _in(value, UCI_FAMILY)): continue assert name not in codes, f"{name} defined twice across the headers" @@ -111,6 +143,20 @@ def _peer_snapshot(): for m in (_PEER_RE.match(l) for l in _read(ASSERTS_TU)) if m} +def _peer_snapshot_expected_name(snapshot_name): + """The peer's own spelling implied by a NET_ERR_PEER_* name. + + NET_ERR_PEER_UCI_SEND_TOO_LONG -> UCI_ERR_SEND_TOO_LONG + NET_ERR_PEER_IP65_UDP_LISTEN -> NET_ERR_IP65_UDP_LISTEN + """ + rest = snapshot_name[len("NET_ERR_PEER_"):] + if rest.startswith("UCI_"): + return "UCI_ERR_" + rest[len("UCI_"):] + if rest.startswith("IP65_"): + return "NET_ERR_IP65_" + rest[len("IP65_"):] + return rest + + def _registered_names(): """Names the asserts TU actually puts through a collision macro.""" names = set() @@ -121,6 +167,23 @@ def _registered_names(): return names +def _macro_peer_refs(): + """{'IP65': {peer names asserted}, 'UCI': {...}} from the macro bodies.""" + refs = {"IP65": set(), "UCI": set()} + current = None + for line in _read(ASSERTS_TU): + m = re.match(r"^\s*\.macro\s+NET_ERR_ASSERT_(IP65|UCI)\b", line) + if m: + current = m.group(1) + continue + if re.match(r"^\s*\.endmacro\b", line): + current = None + continue + if current: + refs[current].update(re.findall(r"\bNET_ERR_PEER_[A-Za-z0-9_]+", line)) + return refs + + def _wireguard_root(): env = os.environ.get("C64_WIREGUARD_ROOT") candidates = [Path(env)] if env else [REPO.parent / "c64-wireguard", @@ -157,7 +220,7 @@ def _peer_registry(root): # -------------------------------------------------------------------------- -# 1-4: structural, no peer checkout needed. +# 1-6: structural. No peer checkout needed; these always run. # -------------------------------------------------------------------------- def test_every_code_is_in_its_family_range(): @@ -178,6 +241,26 @@ def test_every_code_is_registered_in_the_asserts_tu(): f"link-time collision guard silently does not cover them (#184).") +def test_our_codes_are_pairwise_distinct(): + """Two of our own names on one byte is the same defect, intra-repo. + + The assembler cannot express this: its literal pins cover the codes + that existed when they were written, and a NEW duplicate passes every + macro check (range ok, no peer collision) because the value is already + legitimately ours. Caught here instead. + """ + by_value = {} + for name, (value, _path, _family) in sorted(_our_codes().items()): + by_value.setdefault(value, []).append(name) + dupes = {f"${v:02X}": names for v, names in sorted(by_value.items()) + if len(names) > 1} + assert not dupes, ( + f"one value, several names — our own allocations collide: {dupes}. " + f"A published value is never reassigned, and it is never doubled up " + f"either: net_last_error carries one byte and a post-mortem cannot " + f"tell these apart (#184).") + + def test_no_code_of_ours_sits_on_a_peer_owned_value(): peer = _peer_snapshot() by_value = {} @@ -195,13 +278,28 @@ def test_no_code_of_ours_sits_on_a_peer_owned_value(): + ". Allocate in c64-wireguard/src/net_abi.inc first (#184).") -def test_the_mirrored_code_tracks_the_peer_value(): - codes = _our_codes() - peer = _peer_snapshot() - assert MIRRORED in codes, f"{MIRRORED} vanished from uci_errors.inc" - assert peer.get("NET_ERR_PEER_UCI_LONG_READ") == codes[MIRRORED][0], ( - f"{MIRRORED} must mirror c64-wireguard's $8A exactly; it is their " - f"allocation, reserved and never emitted here (#184).") +def test_every_snapshot_entry_is_asserted_by_a_macro(): + """A NET_ERR_PEER_* equate with no .assert is a peer code nothing checks. + + The macro bodies are hand-written, so adding a row to the table without + adding the matching assert line would leave the assembler half quietly + not covering it. + """ + snapshot = _peer_snapshot() + refs = _macro_peer_refs() + missing = [] + for name, value in sorted(snapshot.items(), key=lambda kv: kv[1]): + family = "IP65" if _in(value, IP65_FAMILY) else "UCI" + if name == "NET_ERR_PEER_UCI_LONG_READ": + continue # the mirror: asserted for equality outside the macro + if name not in refs[family]: + missing.append(f"{name} (${value:02X}) missing from " + f"NET_ERR_ASSERT_{family}") + assert not missing, ( + "peer codes in the NET_ERR_PEER_* table that no macro asserts " + "against: " + "; ".join(missing) + + ". Add the .assert line, or the assembler does not actually check " + "that value (#184).") def test_header_prose_lists_match_the_snapshot_table(): @@ -228,29 +326,27 @@ def test_header_prose_lists_match_the_snapshot_table(): # -------------------------------------------------------------------------- -# 5-7: cross-repo drift. Needs a c64-wireguard checkout. +# 7-10: cross-repo drift. Needs a c64-wireguard checkout; a missing one is an +# involuntary skip, i.e. a failure, unless C64_ALLOW_SKIP=1. # -------------------------------------------------------------------------- def _require_peer(): root = _wireguard_root() - if root is None: - try: - import pytest - except ImportError: - return None - pytest.skip( - "no c64-wireguard checkout found, so the snapshot in " - "src/net_err_registry_asserts.s is UNVERIFIED against the " - "canonical registry. Set C64_WIREGUARD_ROOT=/path/to/c64-wireguard " - "(or place it at ../c64-wireguard) to cover this (#184).") + require( + root is not None, + "no c64-wireguard checkout found, so this repo's snapshot of the " + "canonical net_last_error registry is UNVERIFIED. Set " + "C64_WIREGUARD_ROOT=/path/to/c64-wireguard, or place it at " + "../c64-wireguard", + executed=6, total=TOTAL_CHECKS, + certifies=CERTIFIES, + opt_out_env="C64_ALLOW_SKIP", + ) return root -def test_snapshot_matches_the_peer_registry(): +def test_snapshot_values_match_the_peer_registry(): root = _require_peer() - if root is None: - print("SKIP: no c64-wireguard checkout") - return registry = _peer_registry(root) theirs = {v for v, (_n, owner) in registry.items() if owner == "c64-wireguard"} snapshot = set(_peer_snapshot().values()) @@ -263,12 +359,33 @@ def test_snapshot_matches_the_peer_registry(): f"Update src/net_err_registry_asserts.s and both headers (#184).") +def test_snapshot_names_match_the_peer_registry(): + """Values alone are not enough: a RENAME upstream leaves every value + check green while our snapshot, our headers and our diagnostics all + carry a name that no longer exists. That includes the $8A mirror, whose + whole purpose is to carry their name.""" + root = _require_peer() + registry = _peer_registry(root) + wrong = [] + for name, value in sorted(_peer_snapshot().items(), key=lambda kv: kv[1]): + row = registry.get(value) + if row is None: + continue # a value drift; the check above owns it + expected = _peer_snapshot_expected_name(name) + if row[0] != expected: + wrong.append(f"${value:02X}: we call it {expected} " + f"(as {name}), they now call it {row[0]}") + assert not wrong, ( + "c64-wireguard has RENAMED codes our snapshot mirrors: " + + "; ".join(wrong) + + f". Registry: {root}/src/net_abi.inc. Rename ours to match — the " + "value is the contract, the name is how a post-mortem reads it " + "(#184).") + + def test_no_code_of_ours_is_claimed_by_the_peer_registry(): """The finding this whole ticket exists to make impossible to miss.""" root = _require_peer() - if root is None: - print("SKIP: no c64-wireguard checkout") - return registry = _peer_registry(root) bad = [] for name, (value, _path, _family) in sorted(_our_codes().items()): @@ -286,9 +403,6 @@ def test_no_code_of_ours_is_claimed_by_the_peer_registry(): def test_every_code_of_ours_appears_in_the_peer_registry(): root = _require_peer() - if root is None: - print("SKIP: no c64-wireguard checkout") - return registry = _peer_registry(root) unlisted = sorted( f"{name} = ${value:02X}" @@ -302,19 +416,35 @@ def test_every_code_of_ours_appears_in_the_peer_registry(): def main(): - failures = 0 + """Standalone lane. + + `require()` raises SkipPolicyError (an AssertionError) on a missing + peer checkout, so it lands in the FAIL bucket — an involuntary skip is + a failure. With C64_ALLOW_SKIP=1 it raises VoluntarySkip instead, which + is a plain Exception and MUST be named before any broad handler; see + _skip_policy.VoluntarySkip. Catching only AssertionError here is what + let a pytest.skip() BaseException kill this runner mid-suite. + """ + failures = skipped = 0 for name, fn in sorted(globals().items()): if not name.startswith("test_") or not callable(fn): continue try: fn() print(f"PASS {name}") + except VoluntarySkip as exc: + skipped += 1 + print(f"SKIP {name}\n {exc}") except AssertionError as exc: failures += 1 print(f"FAIL {name}\n {exc}") root = _wireguard_root() - print(f"\npeer registry: {root or 'NOT FOUND (cross-repo checks skipped)'}") - print(f"{'FAILED' if failures else 'OK'} — {failures} failure(s)") + print(f"\npeer registry: {root or 'NOT FOUND'}") + if skipped: + print(f"{skipped} check(s) skipped by explicit C64_ALLOW_SKIP=1 opt-out " + f"— this run certifies NOTHING about {CERTIFIES}") + print(f"{'FAILED' if failures else 'OK'} — {failures} failure(s), " + f"{skipped} skipped") return 1 if failures else 0 From 123fccb459664d7c4d8ae093d1734759e897ded8 Mon Sep 17 00:00:00 2001 From: JC-000 <3798556+JC-000@users.noreply.github.com> Date: Mon, 7 Sep 2026 23:33:58 -0500 Subject: [PATCH 3/4] fix(net): catch intra-repo duplicates at BUILD time; scope both directions (#184 review 2) N1 -- "the assembler cannot express this" was a FALSE MECHANISM CLAIM, and it foreclosed the better guard for a round. ca65 expresses it in one line: .macro NET_ERR_CLAIM_VALUE val .ident(.sprintf("NET_ERR_TAKEN_%02X", val)) = 1 .endmacro A second claim on a byte is a redefinition error naming the BYTE. O(n), no hand-maintained list, constant equate so still zero bytes. Called from both assert macros and once for the $8A mirror (which cannot go through a macro that asserts the value differs from every peer code). An intra-repo duplicate now fails the build on all five profiles instead of being caught only by a suite `make` never runs. The claim was wrong in four places and is corrected in all four; the narrower TRUE sentence it was over-generalised from -- the literal pins give distinctness only among codes that existed when they were written -- is kept. While verifying that, a SECOND over-pessimistic claim fell: ca65 evaluates whatever the macro is handed, so a REGISTERED expression-valued equate fires the collision assert exactly like a literal (measured). The residual gap is only an expression-valued code that is ALSO never registered. Corrected in the same four sites; the earlier "passes both halves while colliding" was wrong. N2 -- the decimal widening introduced a false-positive class: an innocent `UCI_HOST_BUF_MAX = 64` produced three red checks telling the author to allocate a buffer size in c64-wireguard's error registry. Gated on the `_ERR_` infix both headers use universally for codes and never for constants. DECISION and its cost: this lets a real code named without `_ERR_` (`UCI_STATUS_FOO = $8C`) past the suite. Accepted because the assemble-time half is name-blind and still catches it the moment it is registered, and because the false positive is a when rather than an if -- buffer sizes favour 64 and 128, which are $40 and $80, the first byte of each family. Named in every scope block. N3 -- moved the opt-out off the shared C64_ALLOW_SKIP (which also gates test_build_flags_stamp.py's toolchain prerequisite) to a dedicated C64_NO_PEER_REGISTRY. One hatch, one door. N4 -- the two headers' scope blocks are parallel again; ip65's kept a round-1 sentence that understated the missing-checkout behaviour. N5 -- inline immediates (`lda #$8C / sta net_last_error`) named as invisible to both halves. Latent: no such site exists. Every scope block now states over-coverage as well as under-coverage. Also: _our_codes()'s bare `assert` became a named RegistryParseError (it vanished under python -O and misattributed to whichever test ran first), and _peer_snapshot_expected_name() gained PEER_NAME_OVERRIDES for a future peer name fitting neither derivation. Keeping cannot_run per the coordinator: the check is applicable and unverified, and exit 0 would say otherwise. PRG sha256 unchanged on all five profiles. Co-Authored-By: Claude Opus 5 (1M context) --- CLAUDE.md | 33 ++++++---- src/net/ip65/ip65_errors.inc | 55 +++++++++++----- src/net/uci/uci_errors.inc | 33 ++++++---- src/net_err_registry_asserts.s | 54 +++++++++++++--- tools/test_net_err_registry.py | 115 +++++++++++++++++++++++++++------ 5 files changed, 224 insertions(+), 66 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index db0ef26..b1bcf1e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -310,20 +310,31 @@ drops a symbol fails the link by name on both backends. Surface: holds the peer's codes as `NET_ERR_PEER_*` equates and `.assert`s (scope `error`, so ca65 settles it before ld65) that no code of ours lands on one, that each is in family range, and that no published value has been - reassigned. Its blind spot — a code never registered in it — is closed - by `tools/test_net_err_registry.py`, which parses the two headers - instead; it also catches two of our own names on one byte (which the - assembler cannot express) and diffs our snapshot's **values and names** - against the live peer file when a c64-wireguard checkout is present + reassigned. **Two of our own names on one byte also fails the build**: + `NET_ERR_CLAIM_VALUE` defines `.ident(.sprintf("NET_ERR_TAKEN_%02X", + val))`, so a second claim on a byte is a ca65 redefinition error naming + the BYTE (`Symbol 'NET_ERR_TAKEN_88' is already defined`) — O(n), no + list to maintain, still zero bytes. It covers only codes passed through + the macros. Its blind spot — a code never registered — is closed by + `tools/test_net_err_registry.py`, which parses the two headers instead + and diffs our snapshot's **values and names** against the live peer file (`C64_WIREGUARD_ROOT`, else `../c64-wireguard`, else `~/Documents/c64-wireguard`). A missing checkout is an **involuntary skip**, so those four checks FAIL rather than pass quietly - (`tools/_skip_policy.py`, #178); `C64_ALLOW_SKIP=1` is the loud opt-out - and still prints the vacuity block. **Scope the guarantee correctly: - both halves are text-level, and recognise only `NAME = $hh`, - `NAME = ddd` and `.define NAME $hh`** — an expression-valued equate - (`UCI_ERR_NEW = UCI_ERR_NO_SOCKET + 4`) passes both while colliding, and - is documented out of scope rather than half-handled. Write literals. The + (`tools/_skip_policy.py`, #178) — a fresh clone with no peer checkout is + RED by design; `C64_NO_PEER_REGISTRY=1` is the loud opt-out (its **own** + variable, deliberately not `C64_ALLOW_SKIP`, which also gates + `test_build_flags_stamp.py`'s toolchain check). **Scope the guarantee + in both directions.** Under-coverage: both halves are text-level and + the **suite** recognises only `NAME = $hh`, `NAME = ddd` and + `.define NAME $hh`. ca65 evaluates an expression-valued code fine once + registered, so the real gap is an expression-valued code that is ALSO + never registered, plus a bare inline `lda #$8C` with no equate at all — + write literals. + Over-coverage: the suite reads whole headers that also hold ordinary + constants, so it gates on the `_ERR_` infix every code uses; without + that a future `UCI_HOST_BUF_MAX = 64` would be reported as an ip65-family + error code needing allocation in c64-wireguard's registry. The `NET_FAMILY_*` bits in `src/net/net_families.inc` are the same cross-repo copy problem and are still unguarded. - Gone, per §13.1: `net_tcp_set_recv_cb` (stub), `net_recv_ready`, diff --git a/src/net/ip65/ip65_errors.inc b/src/net/ip65/ip65_errors.inc index 36ab4b7..bc46dbf 100644 --- a/src/net/ip65/ip65_errors.inc +++ b/src/net/ip65/ip65_errors.inc @@ -31,22 +31,45 @@ ; merge. Comments only, deliberately: an equate here would imply we emit ; them. Their registry is the authority, not this snapshot (#184). ; -; THAT SNAPSHOT IS NOW MACHINE-CHECKED (#184), because prose does not fail a -; build. src/net_err_registry_asserts.s assembles into every build on both -; backends, emits no bytes, and `.assert`s that no code defined here lands -; on a peer-owned value, that each is in the $40-$7F family range, and that -; no published value has been reassigned. tools/test_net_err_registry.py -; parses THIS file, so a code that never gets registered in that TU is -; caught too, and diffs the snapshot against the live peer registry when a -; c64-wireguard checkout is present. Adding a code means all three: their -; registry, the equate here, the registration there. -; -; Narrow, though, and worth knowing before trusting a green run: both -; guards read `NAME = $hh` / `NAME = ddd` / `.define NAME $hh` and nothing -; else, so an EXPRESSION-valued equate passes both while colliding — write -; literals, as every code below does. The cross-repo half needs a peer -; checkout and FAILS without one rather than passing quietly -; (`C64_ALLOW_SKIP=1` opts out loudly). +; THAT SNAPSHOT IS NOW MACHINE-CHECKED (#184). The list above is prose, and +; prose does not fail a build, so two guards stand behind it: +; +; src/net_err_registry_asserts.s assembles into EVERY build, both +; backends, every profile, and emits no bytes. It `.assert`s that no +; code defined here lands on a peer-owned value, that each is in the +; $40-$7F family range, and that no published value has been +; reassigned; NET_ERR_CLAIM_VALUE additionally makes two of our own +; names on one byte a ca65 redefinition error. Minting $46 fails the +; build with a named message instead of linking clean. +; tools/test_net_err_registry.py parses THIS file rather than the +; asserts TU, so a new code that never gets registered there is still +; caught, and diffs our snapshot's values AND names against the live +; c64-wireguard/src/net_abi.inc. +; +; Adding a code: allocate in c64-wireguard/src/net_abi.inc, add the equate +; here, then register it in src/net_err_registry_asserts.s. +; +; WHAT THAT PAIR DOES NOT COVER — the guarantee is real but narrow: +; - **Write the value as a literal.** The suite recognises `NAME = $hh`, +; `NAME = ddd` and `.define NAME $hh` and nothing else. An expression — +; say `NET_ERR_IP65_NEW = NET_ERR_IP65_SEND + 1` — is invisible to it. +; ca65 evaluates such a code fine ONCE REGISTERED, so the gap is the +; narrow one: an expression-valued code that is also never registered +; in src/net_err_registry_asserts.s escapes both halves. Every code +; below is a literal; keep it that way. +; - An inline immediate (`lda #$46 / sta net_last_error`) with no equate +; behind it is invisible to both. No such site exists today. +; - The cross-repo half needs a c64-wireguard checkout. Without one those +; checks FAIL — an involuntary skip is a failure — rather than passing +; quietly; `C64_NO_PEER_REGISTRY=1` is the loud opt-out. +; - It can also go red the OTHER way: the suite reads this whole file, so +; an ordinary constant that happens to land in $40-$7F would look like +; an allocation. It is kept out by the `_ERR_` infix every code here +; uses — so name a new code `NET_ERR_IP65_*`, and expect no help from +; the suite if you do not. +; - Nothing here re-verifies c64-wireguard. A collision found against +; their registry is a cross-repo finding for a human, not something +; either repo may renumber unilaterally. .ifndef IP65_ERRORS_INC_INCLUDED IP65_ERRORS_INC_INCLUDED = 1 diff --git a/src/net/uci/uci_errors.inc b/src/net/uci/uci_errors.inc index 6551cce..a306c76 100644 --- a/src/net/uci/uci_errors.inc +++ b/src/net/uci/uci_errors.inc @@ -41,25 +41,36 @@ ; backends, every profile, and emits no bytes. It carries the peer's ; codes as NET_ERR_PEER_* equates and `.assert`s that no code defined ; here lands on one, that each is in family range, and that no -; published value has been reassigned. Minting $8C fails ca65 with a -; named message instead of linking clean. +; published value has been reassigned; NET_ERR_CLAIM_VALUE +; additionally makes two of our own names on one byte a ca65 +; redefinition error. Minting $8C fails the build with a named +; message instead of linking clean. ; tools/test_net_err_registry.py parses THIS file rather than the ; asserts TU, so a new code that never gets registered there is still -; caught, and diffs our snapshot against the live -; c64-wireguard/src/net_abi.inc when a checkout is present. +; caught, and diffs our snapshot's values AND names against the live +; c64-wireguard/src/net_abi.inc. ; ; Adding a code: allocate in c64-wireguard/src/net_abi.inc, add the equate ; here, then register it in src/net_err_registry_asserts.s. ; ; WHAT THAT PAIR DOES NOT COVER — the guarantee is real but narrow: -; - **Write the value as a literal.** Both guards recognise `NAME = $hh`, -; `NAME = ddd` and `.define NAME $hh`. An expression — say -; `UCI_ERR_NEW = UCI_ERR_NO_SOCKET + 4` — is invisible to a text -; parser and passes BOTH halves while colliding. Every code below is a -; literal; keep it that way. +; - **Write the value as a literal.** The suite recognises `NAME = $hh`, +; `NAME = ddd` and `.define NAME $hh` and nothing else. An expression — +; say `UCI_ERR_NEW = UCI_ERR_NO_SOCKET + 4` — is invisible to it. +; ca65 evaluates such a code fine ONCE REGISTERED, so the gap is the +; narrow one: an expression-valued code that is also never registered +; in src/net_err_registry_asserts.s escapes both halves. Every code +; below is a literal; keep it that way. +; - An inline immediate (`lda #$8C / sta net_last_error`) with no equate +; behind it is invisible to both. No such site exists today. ; - The cross-repo half needs a c64-wireguard checkout. Without one those -; checks fail (an involuntary skip is a failure) rather than passing; -; `C64_ALLOW_SKIP=1` is the loud opt-out. +; checks FAIL — an involuntary skip is a failure — rather than passing +; quietly; `C64_NO_PEER_REGISTRY=1` is the loud opt-out. +; - It can also go red the OTHER way: the suite reads this whole file, so +; an ordinary constant that happens to land in $80-$BF would look like +; an allocation. It is kept out by the `_ERR_` infix every code here +; uses — so name a new code `UCI_ERR_*`, and expect no help from the +; suite if you do not. ; - Nothing here re-verifies c64-wireguard. A collision found against ; their registry is a cross-repo finding for a human, not something ; either repo may renumber unilaterally. diff --git a/src/net_err_registry_asserts.s b/src/net_err_registry_asserts.s index 3829bd2..e823892 100644 --- a/src/net_err_registry_asserts.s +++ b/src/net_err_registry_asserts.s @@ -40,20 +40,28 @@ ; is manual, and a code with no NET_ERR_ASSERT_* line is simply not ; checked here. tools/test_net_err_registry.py covers that by parsing ; the HEADERS instead of this file. -; - This file cannot see two of OUR names on one byte. The literal pins -; below give distinctness only among the codes that existed when they -; were written; a new duplicate passes every macro check, because the -; value is already legitimately ours. Covered by the suite's +; - Two of OUR names on one byte IS caught here, by NET_ERR_CLAIM_VALUE +; below — but only for codes that go through the macros. The literal +; pins give distinctness only among the codes that existed when they +; were written, and the peer-collision asserts do not look at our own +; set at all, so a new duplicate passes both of those; the claim is +; what catches it. An UNREGISTERED duplicate is still the suite's ; test_our_codes_are_pairwise_distinct. ; - This file cannot see the peer repository. Value and name drift there ; are covered by the suite, and only when a checkout is present — a ; missing one is an involuntary skip, so those checks FAIL rather than -; pass quietly (tools/_skip_policy.py; C64_ALLOW_SKIP=1 to opt out). -; - NEITHER half evaluates ca65. The suite recognises `NAME = $hh`, -; `NAME = ddd` and `.define NAME $hh`; an EXPRESSION-valued equate -; (`UCI_ERR_NEW = UCI_ERR_NO_SOCKET + 4`) is out of scope and passes -; both halves while colliding. Declare codes as literals — every code -; in both headers does. +; pass quietly (tools/_skip_policy.py; C64_NO_PEER_REGISTRY=1 to +; opt out). +; - EXPRESSION-valued equates: this file handles them FINE — ca65 +; evaluates whatever the macro is handed, so a registered +; `UCI_ERR_NEW = UCI_ERR_NO_SOCKET + 4` fires the $8C collision assert +; like any literal (measured, not assumed). The suite is the half that +; cannot: it recognises `NAME = $hh`, `NAME = ddd` and +; `.define NAME $hh` only. So the residual gap is narrow and specific — +; an expression-valued code that is ALSO never registered here is +; invisible to both, because the suite's registration check is what +; would otherwise have caught it. Declare codes as literals and the +; question does not arise; every code in both headers does. ; ; MAINTENANCE. Adding a code: allocate it in c64-wireguard/src/net_abi.inc ; FIRST, then in the emitting header here, then register it below. Never @@ -92,7 +100,26 @@ NET_ERR_PEER_UCI_SHORT_READ = $8F ; equate, so ca65 settles these before ld65 is reached and a collision fails ; the build at the offending object rather than at the link. +; ONE VALUE, ONE NAME — enforced at assemble time, O(n), no list to keep. +; Each claimed value defines a symbol named after the value itself, so a +; second claim on the same byte is a ca65 redefinition error: +; +; Error: Symbol 'NET_ERR_TAKEN_88' is already defined +; +; That message names the BYTE, not the pair, so read it as "something else +; already owns $88" and grep both headers for it. The symbol is a constant +; equate, so this costs no bytes like everything else here. +; +; This covers every code passed through the two macros below, plus the $8A +; mirror which claims its byte explicitly. It does NOT cover a code that was +; never registered at all — that stays +; tools/test_net_err_registry.py::test_every_code_is_registered_in_the_asserts_tu. +.macro NET_ERR_CLAIM_VALUE val + .ident(.sprintf("NET_ERR_TAKEN_%02X", val)) = 1 +.endmacro + .macro NET_ERR_ASSERT_IP65 val, name + NET_ERR_CLAIM_VALUE val .assert (val) >= NET_ERR_IP65_FAMILY_LO && (val) <= NET_ERR_IP65_FAMILY_HI, error, .concat(name, ": outside the ip65 family range $40-$7F (c64-wireguard/src/net_abi.inc registry, #184)") .assert (val) <> NET_ERR_PEER_IP65_UDP_LISTEN, error, .concat(name, ": collides with c64-wireguard's $46 NET_ERR_IP65_UDP_LISTEN - allocate in c64-wireguard/src/net_abi.inc first (#184)") .assert (val) <> NET_ERR_PEER_IP65_UDP_SEND, error, .concat(name, ": collides with c64-wireguard's $47 NET_ERR_IP65_UDP_SEND (reserved, never emitted) - allocate in c64-wireguard/src/net_abi.inc first (#184)") @@ -101,6 +128,7 @@ NET_ERR_PEER_UCI_SHORT_READ = $8F .endmacro .macro NET_ERR_ASSERT_UCI val, name + NET_ERR_CLAIM_VALUE val .assert (val) >= NET_ERR_UCI_FAMILY_LO && (val) <= NET_ERR_UCI_FAMILY_HI, error, .concat(name, ": outside the UCI family range $80-$BF (c64-wireguard/src/net_abi.inc registry, #184)") .assert (val) <> NET_ERR_PEER_UCI_LONG_READ, error, .concat(name, ": collides with c64-wireguard's $8A UCI_ERR_LONG_READ - allocate in c64-wireguard/src/net_abi.inc first (#184)") .assert (val) <> NET_ERR_PEER_UCI_SEND_TOO_LONG, error, .concat(name, ": collides with c64-wireguard's $8C UCI_ERR_SEND_TOO_LONG - allocate in c64-wireguard/src/net_abi.inc first (#184)") @@ -146,6 +174,12 @@ NET_ERR_ASSERT_UCI UCI_ERR_BAD_READ_HDR, "UCI_ERR_BAD_READ_HDR" ; test_snapshot_names_match_the_peer_registry respectively. .assert UCI_ERR_LONG_READ = NET_ERR_PEER_UCI_LONG_READ, error, "UCI_ERR_LONG_READ must mirror c64-wireguard's $8A exactly; it is their allocation, reserved and never emitted here (#184)" +; It still claims its byte, so a SECOND name of ours on $8A is a build +; error like any other duplicate. It cannot go through the macro above: +; that one asserts the value differs from every peer code, and $8A is a +; peer code — the whole point of this entry. +NET_ERR_CLAIM_VALUE UCI_ERR_LONG_READ + ; PUBLISHED VALUES, PINNED. The registry's single rule is that a published ; value is never reassigned — not renumbered to close a gap, not reused ; because a code turned out unreachable. These literals are that rule made diff --git a/tools/test_net_err_registry.py b/tools/test_net_err_registry.py index 3606b14..381caa1 100644 --- a/tools/test_net_err_registry.py +++ b/tools/test_net_err_registry.py @@ -22,7 +22,7 @@ on a peer-owned value or a published value is reassigned, and it costs no bytes. Its blind spot is a code that is never registered in it, and it cannot see the peer repository at all. This suite covers exactly those two -gaps, and adds the intra-repo checks the assembler cannot express. +gaps, and adds the intra-repo checks that reach beyond it. ============================================================================= WHAT THIS GUARD DOES **NOT** COVER — read before trusting a green run @@ -35,25 +35,46 @@ NAME = 140 recognised .define NAME $8C recognised -An **expression-valued** equate is NOT recognised and will pass every check -here while colliding: +An **expression-valued** equate is NOT recognised HERE: - UCI_ERR_NEW = UCI_ERR_NO_SOCKET + 4 NOT COVERED + UCI_ERR_NEW = UCI_ERR_NO_SOCKET + 4 not seen by this suite -Evaluating that needs an assembler, not a regex. It is declared out of -scope rather than half-handled: a parser that silently mis-evaluates one of -these would be worse than one that visibly does not try. If you are adding -a code, write it as a literal — every existing code in both headers does. +Evaluating that needs an assembler, not a regex, and a parser that silently +mis-evaluated one would be worse than one that visibly does not try. But +the assembler half is not so limited: ca65 evaluates whatever +NET_ERR_ASSERT_* is handed, so a REGISTERED expression-valued code fires +the collision assert exactly like a literal (measured). The residual gap is +therefore narrow: an expression-valued code that is also never registered +in the asserts TU escapes both halves, because the registration check that +would have caught it is the one in this file, and this file cannot see the +code. Write literals and the question does not arise — every existing code +in both headers is one. **The assembler half covers only registered codes.** Check 2 below is what makes that safe, by failing when a code in the headers has no ``NET_ERR_ASSERT_*`` line. The two halves are only jointly complete for the spellings listed above. +**Inline immediates are invisible to both halves.** A `lda #$8C / sta +net_last_error` with no equate behind it is text neither guard looks at. No +such site exists today — every write goes through a named code — so this is +latent, not live. It is named here because a guard's silence about a shape +it cannot see is indistinguishable from a pass. + **Nothing here validates c64-wireguard.** A code of ours their registry has claimed is reported as a finding for a human to take cross-repo. This suite never edits, and never asserts the correctness of, the peer repository. +THE OTHER DIRECTION — this guard CAN go red on something that is not an +error code. It reads two headers that also hold ordinary constants, and a +value in $40-$BF is not by itself evidence of an allocation. `UCI_STATUS_MAX += 16` is out of range today, but buffer sizes favour 64 and 128, which are +$40 and $80 — the first byte of each family. ERR_NAME_MARKER is the gate +that keeps such a constant out, and the comment on it says what the gate in +turn lets through. If this suite ever tells you to allocate something in +c64-wireguard's registry that is plainly not an error code, that is this +class, and the fix is the gate, not the constant. + ============================================================================= THE CROSS-REPO CHECKS NEED A PEER CHECKOUT, and a missing one is an @@ -62,7 +83,7 @@ Point the suite at a checkout with ``C64_WIREGUARD_ROOT=/path``; the defaults are ``../c64-wireguard`` then ``~/Documents/c64-wireguard``. A lane that genuinely has no peer checkout opts out loudly with -``C64_ALLOW_SKIP=1``, which still prints the full vacuity warning. The six +``C64_NO_PEER_REGISTRY=1``, which still prints the full vacuity warning. The six structural checks run either way. """ @@ -92,6 +113,45 @@ # theirs), and is excluded from the collision sweep by name, never by value. MIRRORED = "UCI_ERR_LONG_READ" +# The env var that opts out of the cross-repo checks. DELIBERATELY NOT the +# repo-wide C64_ALLOW_SKIP: that one also gates test_build_flags_stamp.py's +# "is ca65 on PATH" prerequisite, and someone exporting it in a shell +# profile or CI config to quiet THIS suite would silently quiet a genuinely +# missing toolchain too. One hatch, one door. +OPT_OUT_ENV = "C64_NO_PEER_REGISTRY" + +# N2 -- the false-positive gate. Both headers name every error code with an +# `_ERR_` infix (UCI_ERR_*, NET_ERR_IP65_*) and every non-code constant +# without one (UCI_DATA_QUEUE_MAX, UCI_READ_CHUNK_MAX, UCI_STATUS_MAX, +# IP65_ERRORS_INC_INCLUDED). Without this gate an innocent, correctly +# written `UCI_HOST_BUF_MAX = 64` reads as an error code in the ip65 family +# and produces three red checks telling the author to allocate a buffer +# size in c64-wireguard's error registry. Buffer sizes favour exactly the +# values ($40, $80) that land in these ranges, so that is a when, not an if. +# +# WHAT THE GATE LETS THROUGH, stated plainly: a real error code named +# without `_ERR_` -- say `UCI_STATUS_FOO = $8C` -- is invisible to this +# suite. The assemble-time half still catches it the moment it is +# registered (NET_ERR_CLAIM_VALUE and the peer-collision asserts do not +# look at names at all), and an unregistered one is invisible either way, +# which is the pre-existing limit this does not widen. +ERR_NAME_MARKER = "_ERR_" + +# snapshot name -> the peer's literal spelling, for rows whose name does not +# follow either convention _peer_snapshot_expected_name() derives. Empty +# today; every one of the nine current rows derives cleanly. +PEER_NAME_OVERRIDES = {} + + +class RegistryParseError(AssertionError): + """A header is malformed in a way no individual check should own. + + AssertionError so pytest renders it as a plain failure and the + standalone runner's handler catches it -- but a NAMED one, so it is not + a bare `assert` that vanishes under `python -O` and misattributes to + whichever test happened to call the helper first. + """ + # Declaration spellings we recognise. See the docstring's scope block: an # expression-valued equate is deliberately out of scope. # NAME = $hh | NAME = ddd | .define NAME $hh / ddd @@ -132,7 +192,12 @@ def _our_codes(): value = int(m.group(3), 16) if m.group(3) else int(m.group(4), 10) if not (_in(value, IP65_FAMILY) or _in(value, UCI_FAMILY)): continue - assert name not in codes, f"{name} defined twice across the headers" + if ERR_NAME_MARKER not in name: + continue # not an error code -- see N2 note above + if name in codes: + raise RegistryParseError( + f"{name} is defined in both headers; a net_last_error " + f"code has exactly one home (#184)") codes[name] = (value, path, family) return codes @@ -148,7 +213,14 @@ def _peer_snapshot_expected_name(snapshot_name): NET_ERR_PEER_UCI_SEND_TOO_LONG -> UCI_ERR_SEND_TOO_LONG NET_ERR_PEER_IP65_UDP_LISTEN -> NET_ERR_IP65_UDP_LISTEN + + The two rewrites fit all nine rows today. A future peer name fitting + neither shape would otherwise force a contorted snapshot name or a + spurious red, so PEER_NAME_OVERRIDES is the escape hatch: put the + peer's literal spelling there and keep our name readable. """ + if snapshot_name in PEER_NAME_OVERRIDES: + return PEER_NAME_OVERRIDES[snapshot_name] rest = snapshot_name[len("NET_ERR_PEER_"):] if rest.startswith("UCI_"): return "UCI_ERR_" + rest[len("UCI_"):] @@ -244,10 +316,17 @@ def test_every_code_is_registered_in_the_asserts_tu(): def test_our_codes_are_pairwise_distinct(): """Two of our own names on one byte is the same defect, intra-repo. - The assembler cannot express this: its literal pins cover the codes - that existed when they were written, and a NEW duplicate passes every - macro check (range ok, no peer collision) because the value is already - legitimately ours. Caught here instead. + The assembler DOES catch this for any registered code, via + NET_ERR_CLAIM_VALUE in src/net_err_registry_asserts.s -- a duplicate is + a ca65 redefinition error and fails the build on all five profiles. + (An earlier revision of this file claimed the assembler could not + express it. That was wrong, and it foreclosed the better guard for a + round.) What remains true, and is why this check stays: the literal + pins give distinctness only among the codes that existed when they were + written, and the peer-collision asserts never look at our own set, so + NEITHER of those catches a new duplicate. And an UNREGISTERED duplicate + reaches no macro at all, so the claim never runs -- that case is this + check's alone. """ by_value = {} for name, (value, _path, _family) in sorted(_our_codes().items()): @@ -327,7 +406,7 @@ def test_header_prose_lists_match_the_snapshot_table(): # -------------------------------------------------------------------------- # 7-10: cross-repo drift. Needs a c64-wireguard checkout; a missing one is an -# involuntary skip, i.e. a failure, unless C64_ALLOW_SKIP=1. +# involuntary skip, i.e. a failure, unless C64_NO_PEER_REGISTRY=1. # -------------------------------------------------------------------------- def _require_peer(): @@ -340,7 +419,7 @@ def _require_peer(): "../c64-wireguard", executed=6, total=TOTAL_CHECKS, certifies=CERTIFIES, - opt_out_env="C64_ALLOW_SKIP", + opt_out_env=OPT_OUT_ENV, ) return root @@ -420,7 +499,7 @@ def main(): `require()` raises SkipPolicyError (an AssertionError) on a missing peer checkout, so it lands in the FAIL bucket — an involuntary skip is - a failure. With C64_ALLOW_SKIP=1 it raises VoluntarySkip instead, which + a failure. With C64_NO_PEER_REGISTRY=1 it raises VoluntarySkip instead, which is a plain Exception and MUST be named before any broad handler; see _skip_policy.VoluntarySkip. Catching only AssertionError here is what let a pytest.skip() BaseException kill this runner mid-suite. @@ -441,7 +520,7 @@ def main(): root = _wireguard_root() print(f"\npeer registry: {root or 'NOT FOUND'}") if skipped: - print(f"{skipped} check(s) skipped by explicit C64_ALLOW_SKIP=1 opt-out " + print(f"{skipped} check(s) skipped by explicit {OPT_OUT_ENV}=1 opt-out " f"— this run certifies NOTHING about {CERTIFIES}") print(f"{'FAILED' if failures else 'OK'} — {failures} failure(s), " f"{skipped} skipped") From 5bb4ad9f31e29609078c9bf76d7eb1f01477463e Mon Sep 17 00:00:00 2001 From: JC-000 <3798556+JC-000@users.noreply.github.com> Date: Mon, 7 Sep 2026 23:43:13 -0500 Subject: [PATCH 4/4] docs(net): correct the registry guard's blast-radius and gap-list claims (#184 review 3) Text only -- comments, docstrings and CLAUDE.md prose. No executable line in either guard changed; verified by filtering the .s and .py diffs, and by rebuilding all five profiles to unmoved PRG hashes. N6 -- the fourth untested mechanism claim in this area, and the one that mattered. The ERR_NAME_MARKER comment said a non-`_ERR_`-named unregistered code "is invisible either way, which is the pre-existing limit this does not widen". Measured on the two commits either side of the gate, with the same mutation (an unregistered, colliding UCI_STATUS_FOO = $8C): 3 failures at 53a35bb, 0 at 123fccb. The gate CREATED that class; an `_ERR_`-named unregistered code is caught at both revisions, so nothing was being restated. Corrected to say so. The decision two lines above is unchanged and still right -- only the blast radius was wrong. N7 -- CLAUDE.md listed under-coverage as two members while both headers and the suite docstring named three, and CLAUDE.md is what a future reader consults. It now enumerates all three, each with the "invisible to BOTH halves" qualifier that is what actually makes something a blind spot, and names (2) as a gap the gate created rather than inherited. N8 -- RegistryParseError's docstring claimed to have fixed the misattribution as well as the `python -O` erasure. Only the erasure and the naming are fixed: the exception is still raised from a helper and surfaces under every test that calls _our_codes(). Clause dropped, limitation stated instead. N9 -- kept NET_ERR_CLAIM_VALUE UCI_ERR_LONG_READ and recorded why, so the next reader does not re-derive that it changes no outcome: it does not, and was checked. It stays to keep the invariant total -- every code this repo defines claims its byte, no exception to carry in your head -- and to give the next macro-exempt code an obvious home. Co-Authored-By: Claude Opus 5 (1M context) --- CLAUDE.md | 20 ++++++++++++-------- src/net_err_registry_asserts.s | 17 +++++++++++++---- tools/test_net_err_registry.py | 21 ++++++++++++++++----- 3 files changed, 41 insertions(+), 17 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index b1bcf1e..833f106 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -325,15 +325,19 @@ drops a symbol fails the link by name on both backends. Surface: RED by design; `C64_NO_PEER_REGISTRY=1` is the loud opt-out (its **own** variable, deliberately not `C64_ALLOW_SKIP`, which also gates `test_build_flags_stamp.py`'s toolchain check). **Scope the guarantee - in both directions.** Under-coverage: both halves are text-level and - the **suite** recognises only `NAME = $hh`, `NAME = ddd` and - `.define NAME $hh`. ca65 evaluates an expression-valued code fine once - registered, so the real gap is an expression-valued code that is ALSO - never registered, plus a bare inline `lda #$8C` with no equate at all — - write literals. + in both directions.** Under-coverage is **three** things, and a code + reaches a blind spot only by being invisible to *both* halves: (1) an + expression-valued code that is ALSO never registered — the **suite** + reads only `NAME = $hh`, `NAME = ddd` and `.define NAME $hh`, while ca65 + evaluates an expression fine once registered, so neither alone is the + gap; (2) a code named without the `_ERR_` infix that is ALSO never + registered — a gap the over-coverage gate below CREATED rather than + inherited, and the accepted cost of it; (3) a bare inline `lda #$8C` + with no equate at all, which no text guard sees (latent — no such site + exists). Write literals, name them `*_ERR_*`, register them. Over-coverage: the suite reads whole headers that also hold ordinary - constants, so it gates on the `_ERR_` infix every code uses; without - that a future `UCI_HOST_BUF_MAX = 64` would be reported as an ip65-family + constants, so it gates on that same `_ERR_` infix; without it a future + `UCI_HOST_BUF_MAX = 64` would be reported as an ip65-family error code needing allocation in c64-wireguard's registry. The `NET_FAMILY_*` bits in `src/net/net_families.inc` are the same cross-repo copy problem and are still unguarded. diff --git a/src/net_err_registry_asserts.s b/src/net_err_registry_asserts.s index e823892..6d7dc66 100644 --- a/src/net_err_registry_asserts.s +++ b/src/net_err_registry_asserts.s @@ -174,10 +174,19 @@ NET_ERR_ASSERT_UCI UCI_ERR_BAD_READ_HDR, "UCI_ERR_BAD_READ_HDR" ; test_snapshot_names_match_the_peer_registry respectively. .assert UCI_ERR_LONG_READ = NET_ERR_PEER_UCI_LONG_READ, error, "UCI_ERR_LONG_READ must mirror c64-wireguard's $8A exactly; it is their allocation, reserved and never emitted here (#184)" -; It still claims its byte, so a SECOND name of ours on $8A is a build -; error like any other duplicate. It cannot go through the macro above: -; that one asserts the value differs from every peer code, and $8A is a -; peer code — the whole point of this entry. +; It still claims its byte. It cannot go through the macro above: that one +; asserts the value differs from every peer code, and $8A IS a peer code — +; the whole point of this entry. +; +; WHY KEEP THIS LINE, since it changes no outcome. It was checked: a second +; name of ours on $8A trips the macro's own $8A peer-collision assert first +; if it is registered, and test_our_codes_are_pairwise_distinct if it is +; not, so deleting this claim would fail exactly nothing and no check would +; notice. It stays because it makes the invariant total — EVERY code this +; repo defines claims its byte, with no exceptions to carry in your head — +; and a rule with one silent exception is the kind that rots. It is also +; the one hand-maintained claim site, so if you add another code that +; cannot go through a macro, it goes here beside this note. NET_ERR_CLAIM_VALUE UCI_ERR_LONG_READ ; PUBLISHED VALUES, PINNED. The registry's single rule is that a published diff --git a/tools/test_net_err_registry.py b/tools/test_net_err_registry.py index 381caa1..7cf1b41 100644 --- a/tools/test_net_err_registry.py +++ b/tools/test_net_err_registry.py @@ -133,8 +133,16 @@ # without `_ERR_` -- say `UCI_STATUS_FOO = $8C` -- is invisible to this # suite. The assemble-time half still catches it the moment it is # registered (NET_ERR_CLAIM_VALUE and the peer-collision asserts do not -# look at names at all), and an unregistered one is invisible either way, -# which is the pre-existing limit this does not widen. +# look at names at all). +# +# THE GATE DOES WIDEN A GAP, and it is worth being exact about which. A +# code that is BOTH non-`_ERR_`-named AND never registered is now invisible +# to both halves. That is a NEW class, not a restatement of an old one: an +# `_ERR_`-named unregistered code is caught here at every revision, and +# before the gate this suite caught the non-`_ERR_` unregistered case three +# ways (measured on the two commits either side of it -- 3 failures before, +# 0 after). The decision above still stands; the cost is one new blind +# spot, not zero. ERR_NAME_MARKER = "_ERR_" # snapshot name -> the peer's literal spelling, for rows whose name does not @@ -147,9 +155,12 @@ class RegistryParseError(AssertionError): """A header is malformed in a way no individual check should own. AssertionError so pytest renders it as a plain failure and the - standalone runner's handler catches it -- but a NAMED one, so it is not - a bare `assert` that vanishes under `python -O` and misattributes to - whichever test happened to call the helper first. + standalone runner's handler catches it -- but a NAMED one, so it does + not vanish under `python -O` the way the bare `assert` it replaced did. + + It is still raised from a helper, so it surfaces under EVERY test that + calls `_our_codes()` and none of them owns the problem. That is not + fixed; the message is self-identifying, which is why it is tolerable. """ # Declaration spellings we recognise. See the docstring's scope block: an