fix(registry): report the protocol-name collision register_protocol hid (#675)

[JarryShaw]

Fixes #675.

The defect, re-verified on b34f132f6

The issue was measured on 0c7f2b7c9; main has moved four merges since, so I re-ran it on
this PR's base. The assignment has drifted from line 150 to line 160, but the defect is
unchanged. Measured with the worktree at sys.path[0], the editable finder stripped from
sys.meta_path, and pcapkit.__file__ asserted before anything else imported:

tree: .../worktrees/agent-ae6e2801f842b8051/pcapkit/__init__.py

before:                                __proto__[HTTP] = <class 'pcapkit.protocols.application.http.HTTP'>
after register_protocol(httpv2.HTTP):  __proto__[HTTP] = <class 'pcapkit.protocols.application.httpv2.HTTP'>
warnings raised: NONE
after register_protocol(httpv1.HTTP):  __proto__[HTTP] = <class 'pcapkit.protocols.application.httpv1.HTTP'>
warnings raised: NONE

distinct keys the three classes land on: ['HTTP']

The consequence is reachable straight through public API in the same run:
ProtocolBase.expand_comp('HTTP') resolved to pcapkit.protocols.application.httpv1.HTTP
afterwards — a bare-name lookup silently returning a different class than it did before.

Two facts the issue did not state, both of which matter for the fix:

1. The three built-in HTTP classes are ProtocolBase subclasses but not subclasses of the
   public Protocol. Protocol.__init_subclass__ is what calls register_protocol(cls)
   unconditionally, so it never fires for them — which is why a plain import pcapkit is silent.
   Measured with the fix applied: 0 RegistryWarnings during import pcapkit.
2. That same __init_subclass__ makes the collision reachable with no registry call in the
   user's code at all. Defining class HTTP(Protocol) displaces the built-in entry. That is the
   sharpest form of the bug; it now warns, and there is a test for it.

The RegistryWarning claim — confirmed on the number, corrected on the reason

The issue says register_protocol is "the one registrar in pcapkit/foundation/registry/ with
zero RegistryWarning uses". The count is right, the explanation is not, and the corrected
version is a stronger argument for fixing it here:

File	RegistryWarning uses
pcapkit/foundation/registry/__init__.py	0
pcapkit/foundation/registry/foundation.py	0
pcapkit/foundation/registry/protocols.py	0

rg 'warn\(' pcapkit/foundation/registry/ returns nothing at all — no function in this
package warns about anything, so "its siblings warn and this one does not" is not true of the code
in this package. The siblings warn by delegating: register_tcp forwards to
Transport.register, register_ipv4_option to IPv4.register_option, and so on, and it is those
classmethods that carry if code in cls.__xxx__: warn(..., RegistryWarning).

So the real outlier property is sharper than the issue states: register_protocol is the only
keyed registrar in the package — and the only __name__.upper()-keyed registry anywhere in
pcapkit/ — that mutates its target dict with no guard and no delegate that could supply one.
There is no classmethod behind the module-level dict pcapkit.protocols.__proto__, so the guard
has to be inline here. That is what this PR does.

Why fix (1) and not fix (2)

Every reader of the registry, and what a unique key would do to each:

Reader	Key it looks up	Behaviour under a unique key
ProtocolBase.expand_comp (protocols/protocol.py:697)	value.upper() from a caller string	Silent miss. Falls back to comp = (value.upper(),). Breaks alias classes whose own name is absent from id() — IPsec.id() returns ('AH', 'ESP'), so frame['IPsec'] would raise ProtocolNotFound instead of matching an ESP layer.
ProtocolBase.__getitem__ / __contains__ / _check_term_threshold	via expand_comp	Inherits the above; packet['HTTP'], 'HTTP' in packet and extract(protocol=...) all ride on it.
ProtoChain.index / count / __contains__ (corekit/protochain.py:113,133,207)	via expand_comp	Inherits the above.
ReassemblyMeta.protocol (foundation/reassembly/reassembly.py:77)	cls.name.upper()	Silent miss → returns Raw as "the protocol this reassembly tracks".
TraceFlowMeta.protocol (foundation/traceflow/traceflow.py:78)	cls.name.upper()	Silent miss → returns Raw.
PayloadField.protocol setter (corekit/fields/misc.py:267)	protocol verbatim, no .upper()	Silent miss → None, then falls back to Raw.
pcapkit/protocols/__init__.py:74-75	name.upper() over __all__	The other writer; would have to be re-keyed in lockstep.

Three things make (2) infeasible in this change:

• Not one reader raises on a miss. They all degrade silently to a bare string or to Raw, so
  a re-keying would itself be a silent behaviour change — the same failure mode as the bug.
• expand_comp takes a bare name by construction. It is handed a user string like 'HTTP',
  so a qualified key does not merely move the lookup, it removes the ability to perform it. That
  is a public contract change: docs/source/pcapkit/protocols/index.rst:154 documents
  pcapkit.protocols.__proto__ with autodata, cross-referenced to register_protocol.
• The readers live in files this change must not touch — pcapkit/protocols/protocol.py in
  particular is owned by other open work right now.

So (1), with (2) recorded in the docstring as belonging to #514 rather than quietly dropped.

The one deliberate departure from house style

The siblings guard on mere presence (if code in cls.__proto__). This guard reads "present
and the incumbent is a different class", because the sibling keys are caller-supplied codes
while this key is derived from the class, and this function is the funnel all nine wrapper
registrars end in. register_tcp(p1, MyProto) followed by register_udp(p2, MyProto) — one class
under two codes, supported and documented — reaches it twice with the same class and nothing
displaced. A presence-only guard would warn about an overwrite that overwrote nothing, and the
wholesale RegistryWarning filter that invites is exactly what would then hide the real
collision. test_register_protocol_stays_quiet_when_nothing_is_displaced pins this, and
test_sibling_registries_still_warn_on_an_identical_re_registration pins that the siblings were
not relaxed to match.

Evidence

Exit codes read from files, never from a pipeline. The PASSED-line-with-SUBFAILED-subtests
trap appeared in the "before" run exactly as expected — the per-test line read PASSED while both
its subtests failed — so the summary line is the thing to read.

Before (pristine protocols.py, new tests) — rc=1:

FAILED    ...::test_a_protocol_subclass_shadowing_a_builtin_name_warns
SUBFAILED(replacing='pcapkit.protocols.application.http')    ...::test_register_protocol_warns_when_a_colliding_name_overwrites
SUBFAILED(replacing='pcapkit.protocols.application.httpv2')  ...::test_register_protocol_warns_when_a_colliding_name_overwrites
3 failed, 7 passed, 75 subtests passed

After (fixed) — rc=0:

8 passed, 77 subtests passed

Nothing else perturbed. Every test file that reads the registry by bare name, defines a
Protocol subclass, asserts "no warnings", or checks docstrings — rc=0:

tests/protocols/test_registry_runtime.py   tests/protocols/test_protocol_base_unit.py
tests/protocols/test_construction_keyword_check_unit.py
tests/protocols/test_protocol_code_registration_unit.py
tests/protocols/internet/test_ipv4_unit.py tests/protocols/internet/test_hip_unit.py
tests/protocols/schema/test_schema_unit.py tests/protocols/application/test_ngap_unit.py
tests/protocols/internet/test_esp_unit.py  tests/utilities/test_warning_filters.py
tests/project/test_public_api.py           tests/test_docstring_contract.py
=> 206 passed, 654 subtests passed

tests/protocols/test_registry_runtime.py is the pre-existing file asserting that the sibling
registrars warn on overwrite; it passes untouched.

tests/foundation/ overall: 238 passed / 11 skipped / 388 subtests, against a baseline of
234 passed / 381 subtests, with the same single pre-existing failure noted below.

Coverage did not go backwards. pcapkit/foundation/registry/protocols.py, same test scope
(tests/foundation/registry/) both times, via coverage run -m pytest:

	Stmts	Miss	Branch	BrPart	Cover
before	270	27	130	0	88%
after	275	27	132	0	88%

+5 statements and +2 branches with misses flat at 27 — every line and branch added is
executed. The missing ranges are the same regions shifted by the docstring's added lines
(198-221 → 254-277, 290-292 → 346-348, 944-953 → 1000-1009). In that directory tests went
9 → 13 and subtests 80 → 87.

Lint unchanged. Project PYLINT_FLAGS on the module, pristine vs fixed, identical message
counts: 10 C0301, 1 E0013, 3 R0022, 1 W0012, 4 W0404 — zero new findings. The isort complaint
on this file is pre-existing (reproduced on the pristine copy) and does not involve the import I
added.

Not done here, deliberately

• pcapkit/foundation/registry/protocols.py:36 is left alone. Narrowing that gate would break
  user classes subclassing ProtocolBase only. Verified rather than trusted:
  issubclass(Protocol, ProtocolBase) is True, issubclass(ProtocolBase, Protocol) is False,
  and a ProtocolBase-only subclass is accepted by the gate today. It stays wide.
• The collision is reported, not resolved. Re-keying belongs to Design: adopt the EnumMeta/EnumSchema opt-in registration pattern for Protocol, Engine, Reassembly and TraceFlow #514, and Profile by self time: aenum.extend_enum is 16.7% of extraction, dispatch and imports under 1% #575 is gated behind
  that. Per Design: adopt the EnumMeta/EnumSchema opt-in registration pattern for Protocol, Engine, Reassembly and TraceFlow #514's part-(c) sequencing this is c1, the step the alias work is meant to follow — so
  this PR constrains Design: adopt the EnumMeta/EnumSchema opt-in registration pattern for Protocol, Engine, Reassembly and TraceFlow #514 by fixing the key space's observability while leaving its format
  untouched, which is what keeps Design: adopt the EnumMeta/EnumSchema opt-in registration pattern for Protocol, Engine, Reassembly and TraceFlow #514 free to choose whatever key it wants.
• tests/foundation/reassembly/test_tcp_runtime.py::TCPReassemblyRuntimeTests::test_sample_capture_reassembles_every_stream_byte_exactly
  fails in this environment before and after, with FileNotFoundError: sample capture 'test.pcap' not found — the generated sample captures are not committed. Environmental,
  pre-existing and unrelated; confirmed against the pristine tree.

Labels

fix + test. Not breaking: this repo defines that label as "Alters public API or wire
output", and nothing here does — no signature change, no registry-format change, the overwrite
still happens with the same result, and import pcapkit gains zero warnings. The caveat worth
stating rather than burying: a downstream project running under -W error that today shadows a
built-in protocol name would now get an exception where it previously got silence. That is the
intended point of the change, and it matches what every sibling registry has always done, so I
read it as fix rather than breaking — say the word if you weight it the other way and I will
add the label.

I am not claiming CI green.

[JarryShaw]

Filed #682 for the residue this PR deliberately leaves: the key space is still not unique, so the three HTTP classes still share one key and the last registration still wins — it is now merely audible rather than silent. #682 carries the full reader inventory and what each reader would do under a changed key, so that analysis does not have to be re-derived, and it defers the choice of key to #514 rather than ahead of it.

Two corrections to my own evidence above, for the record:

• I initially ran the cross-checks with -p no:randomly, which implied test-order randomisation was in play. It is not — pytest-randomly is not installed in this environment, so that flag was a no-op and those runs were all the same fixed order. They show repeatability, not order-independence.
• Order-independence was therefore verified properly instead, by explicit node IDs in one process: the four new mutating tests first, then the pre-existing tests that assert exact registry identity by bare name (test_esp_unit on __proto__['ESP'], test_ngap_unit on __proto__['NGAP'], plus test_protocol_base_unit). rc=0, 70 passed / 68 subtests — so the addCleanup restoration, absence included, holds against a later reader in the same process.

[JarryShaw]

GOOD TO GO

Independent cross-review on a different model (Claude Sonnet) from the one that wrote the
change, briefed to falsify rather than to bless, running read-only. No substitution was needed —
the intended model ran. Recording the verdict here because nothing in GitHub tracks it otherwise.

All eight load-bearing claims came back CONFIRMED, each re-derived independently rather than
taken from the PR description.

Claim	Verdict	How it was established independently
1. No reader of the name registry breaks	CONFIRMED	Re-enumerated the readers from scratch and found the PR's table complete — exactly four direct readers (expand_comp, ReassemblyMeta.protocol, TraceFlowMeta.protocol, PayloadField.protocol), with __getitem__ / __contains__ / _check_term_threshold / ProtoChain.index,count,__contains__ reaching the dict only via expand_comp. The final protocol_registry[name] = protocol still runs unconditionally, so no reader's returned value changes.
2. Sibling registrars unchanged	CONFIRMED	Read all five sibling register methods (ProtocolBase, Transport, Link, Internet, Frame) — all still presence-only if code in cls.__proto__: warn(...). tests/protocols/test_registry_runtime.py → rc=0, 9 passed.
3. The "different class" guard is right, not a bug	CONFIRMED, one latent fragility noted	Could not construct a case where the identity check hides a real collision — two genuinely different classes are different objects by definition. Also chased the None question independently: the only two writers into this dict store real classes, so None is never a value. Flagged as a latent fragility only if future code ever used None as an "unregistered" sentinel.
4. import pcapkit emits zero new warnings	CONFIRMED by its own measurement	Reproduced with the editable finder stripped and pcapkit.__file__ asserted to this worktree: 1 warning total, 0 RegistryWarning (the one being an unrelated DeprecationWarning: VueJS is deprecated). Independently confirmed the structural reason via issubclass(...) checks.
5. Failing-then-passing is real	CONFIRMED on the passing half; the failing half proved by inspection	Reproduced rc=0, 8 passed / 77 subtests, matching exactly. Declined to revert a tracked file to reproduce the "before" run, correctly respecting its read-only mandate, and instead showed the failure is mechanically forced: warn(...) is the only source of RegistryWarning, so on pristine code the assertion is necessarily 0 != 1.
6. Test cleanup cannot corrupt siblings	CONFIRMED	Verified _guard_registry guards every key touched, absence included. Traced class HTTP(Protocol) through __init_subclass__: with code=None it skips register_protocol_code entirely, ProtocolMeta is an empty metaclass, and __schema__/__data__ land on the throwaway class — nothing shared is mutated.
7. No unintended narrowing of line 36	CONFIRMED	Line 36 untouched; issubclass(Protocol, ProtocolBase) is True and the reverse False, so the gate stays wide.
8. Warning message correct and actionable	CONFIRMED	ProtocolMeta defines no __repr__, so repr() always renders <class 'module.Name'> and the three HTTP classes are always distinguishable. Also confirmed a ModuleDescriptor can never be the incumbent — every wrapper resolves .klass before calling in.

It also confirmed the breaking omission is right under this repo's definition, found no false or
self-contradicting statement in the new docstring, and verified the Sphinx trap was avoided: the
docstring uses :class: roles for ReassemblyMeta/TraceFlowMeta, which resolve, rather than
:attr: roles on their members, which would dangle under :no-members:.

What it disputed, and my response

One fair criticism, which I am recording rather than folding away: my "nothing else perturbed"
list omitted the two test files most directly about the defect — test_http_unit.py and
test_http_runtime.py. That was a real gap in curation. I have since run both myself rather than
relying on the review:

tests/protocols/application/test_http_unit.py + test_http_runtime.py
rc=1  ->  5 failed, 29 passed, 15 subtests passed
grep -c 'FileNotFoundError: sample capture' = 5

All five failures are the same environmental cause already disclosed for test_tcp_runtime.py, and
the count of that error equals the count of failures, so no failure has any other cause. Made
airtight: examples/captures/http.pcap is absent from the tree and is not among the tracked
files under examples/captures/, the generated captures not being committed. test_http_unit.py —
the file that actually exercises the HTTP/1 vs HTTP/2 classes — passes clean. The reviewer
independently hit the identical signature in test_ip_runtime.py and test_ipv6_extension_runtime.py
too, all the same missing-fixture gap.

What the review could not verify

Stated plainly rather than implied: it did not execute the pre-fix "before" run (it argued that
half by inspection, for the read-only reason above), and it did not re-run the lint, isort or
coverage tables in the PR body or a full Sphinx nitpick build. Those remain my measurements, not
independently reproduced.

No disagreement was raised that changes the merge decision. Still unpublished and awaiting your
review — I am not claiming CI green.