feat(protocols): opt-in code= registration with enum-type inference for Protocol/ProtocolBase

[JarryShaw]

Summary

Part (b) of #514: the same opt-in treatment PR #547 gave Engine, Reassembly, TraceFlow and Dumper, extended to Protocol/ProtocolBase's next-layer dispatch registries (Link.__proto__, Internet.__proto__, TCP.__proto__, UDP.__proto__, SCTP.__proto__, Frame.__proto__, PCAPNG.__proto__). Protocol was held back from #547 because giving it a registration keyword meant designing what that keyword should mean -- the enum-type inference settled across the issue thread (Q3/Q11/Q12).

• ProtocolBase/Protocol.__init_subclass__ gain a code= keyword. Opt-in: omitting it leaves the class unregistered, exactly as today -- no built-in protocol class passes code=, so nothing the library ships moves.
• code accepts a bare enum member, whose type infers the destination: EtherType -> Link, TransType -> Internet, PayloadProtocolIdentifier -> SCTP, LinkType -> both Frame and PCAPNG (mirroring what register_linktype already does by hand). A raw int (e.g. a TCP/UDP port) has no inferable type and must name its destination explicitly via {destination: key}; either form may appear in an iterable to register one class into several registries from one declaration -- e.g. an L2TP subclass reachable both by IP protocol number and by a UDP port.
• The explicit mapping form is accepted even for a key whose type could be inferred (Q12) -- refusing it would punish being clearer than required.
• Inference refuses rather than guesses: an enum type absent from the table raises RegistryError.
• Unrecognised class keywords raise UnsupportedCall, matching feat(foundation): make subclass registration opt-in for Engine, Reassembly, TraceFlow and Dumper (#514) #547's guard.
• New pcapkit.foundation.registry.protocols.register_protocol_code, mirrored into the __all__ of pcapkit.foundation.registry, pcapkit.foundation and pcapkit.all (each hand-lists every registry function; pcapkit.foundation.registry is one of the three packages tests/project/test_public_api.py holds to the stricter "every public attribute is exported" check).

Verified against the current tree, not just the issue's measurement: re-ran the key-type enumeration on 8cfd6ab01 and it still holds exactly --

key type	destination(s)	inferable?
EtherType	Link	yes
TransType	Internet	yes
PayloadProtocolIdentifier	SCTP	yes
LinkType	Frame and PCAPNG	yes
raw int	Link, TCP, UDP	no

Backward compatibility, measured rather than assumed: Protocol/ProtocolBase descendant counts are 0/43 both before and after (matches the issue's own measurement), and the byte contents of all seven __proto__ tables are identical before and after -- dumped, diffed, confirmed unchanged.

Scope

Deliberately not included, per the issue thread's own sequencing:

• IP protocol 115 (TransType.L2TP) is registered nowhere, so an L2TP-over-IP capture falls through to Raw #548 (TransType.L2TP registered nowhere) is now unblocked -- it splits cleanly along this PR's rule: TransType.L2TP infers into Internet, while the UDP-1701 encapsulation needs an explicit destination, so the fix becomes a one-declaration code=[TransType.L2TP, {UDP: 1701}]. Left for its own issue/PR.
• Part (c), unwinding the ~76 *Base as <Public> alias imports, is separate follow-up work and does not depend on this PR landing first in any blocking way -- it depends on part (a) (feat(foundation): make subclass registration opt-in for Engine, Reassembly, TraceFlow and Dumper (#514) #547, already merged), not on this one.
• The registry-ownership enforcement floated early in the issue thread (raising when .register() is called through an inherited table) is not implemented here: this PR's register_protocol_code only ever calls .register() on the fixed inference-table destinations or an explicitly-named destination class, never through an arbitrary subclass's inherited __proto__, so the ownership gap doesn't affect this mechanism's correctness. Flagging it since the issue thread's later comments group it under "part (b)"; happy to add it if wanted, but the design section handed to me for this PR didn't ask for it.

Test plan

• New unit tests in tests/protocols/test_protocol_code_registration_unit.py (19 cases): opt-in gating, all four inference destinations, the LinkType double fan-out, the raw-int refusal, the explicit-dict form (single and dual destination), the mixed-iterable (L2TPv2) shape, Q12's explicit-for-inferable acceptance, an unknown-enum-type refusal, unrecognised-keyword rejection (including the name=/ABCMeta version-pinned case), and two real end-to-end registrations through a class statement (via mock.patch.dict so nothing leaks into the shared __proto__ tables).
• Proven to fail without the fix: 18 of 19 new tests fail against the pre-fix source (the 19th is a before/after invariant that must hold both ways); all 19 pass with the fix.
• tests/protocols/test_dispatch_registry_unit.py, test_registry_runtime.py, test_protocol_base_unit.py, test_schema_unit.py, test_enum_schema_registry_unit.py: all pass unchanged (50 passed, 94 subtests).
• tests/project/test_public_api.py, test_documentation_claims.py, test_changelog_md.py: all pass (47 passed, 469 subtests) -- confirms the __all__ mirroring is complete and CHANGELOG.md is in step.
• Full unit tier (pytest tests, samples regenerated first via examples/generators/make_samples.py): 1279 passed, 17 skipped, 2850 subtests passed, exit 0.
• mypy on the two edited source files: 4 errors both before and after (same two pre-existing issues, at shifted line numbers) -- unchanged.
• pylint targeted check (unused-import/undefined-variable/unused-variable/no-member): clean, 10.00/10.

[JarryShaw]

✅ GOOD TO MERGE

Cross-model review (Opus 5; the PR was authored on Sonnet), head 3f8820ade. One commit on current main 8cfd6ab01.

This is the best-evidenced PR of the batch, and I could not break the design. Everything load-bearing I re-derived independently rather than accepting:

• Inference matches the documented table exactly, measured by declaring real subclasses through genuine class statements and diffing all seven __proto__ tables before and after: EtherType→Link, TransType→Internet, PayloadProtocolIdentifier→SCTP, LinkType→Frame and PCAPNG. No stray writes into any other registry.
• Inference genuinely refuses rather than falling through — this was the property I most wanted to break, and it holds. Six bad inputs (bare int port, an AppType member whose type is deliberately absent from the table, str, bytes, float, None inside a list) each raise RegistryError and touch no registry at all. The messages name the offending key and suggest the explicit form. Nothing silently picks a registry.
• Backward compatibility re-derived, not taken on trust. I dumped Protocol/ProtocolBase descendant counts and all seven __proto__ tables in a canonical sorted form on main and on this head, in separate fresh interpreters with pcapkit.all imported. Descendant counts 0/43 on both, and the two dumps (38 registry entries) are byte-identical — same SHA256 8f4ef56585cf395e8409a70d44a2cf02b2b9f2f2deeb1e79a63a7341403e832f. Nothing the library ships moves.
• The Python 3.10 ABCMeta trap is avoided, and the test that pins it is correctly version-pinned. Measured on a real 3.10.21 and on 3.14.7, through real class statements rather than type(...): code= is accepted on both; name=, mcls=, bases= and namespace= all raise a raw TypeError: ABCMeta.__new__() got multiple values for argument '…' on 3.10 but UnsupportedCall on 3.14 — exactly the sys.version_info >= (3, 11) split the test encodes. ProtocolMeta's MRO really does include ABCMeta, so the hazard was real and code sidesteps it.
• No hasattr-based registry resolution is introduced. The diff adds hasattr in exactly two places, both in tests checking __schema__/__data__. Resolution is destination.register(key, protocol), delegating wholly to the existing register classmethods — so the ownership guard, whenever it lands there, is inherited by this mechanism with zero changes. The trap is not built in.
• 18 of 19 new tests fail against the pre-fix sources (exit 1 from a file), and the one that passes both ways is test_backward_compatible_dispatch_tables_are_unaffected — precisely the invariant the body says must hold in both directions. All 19 pass with the fix (exit 0).

On the ownership question you asked about: it is orthogonal, it does not make the fix harder, and this can merge before the ownership work. But one sentence in the Scope section is wrong and should be corrected before merge, because it is what an owner would rely on:

register_protocol_code only ever calls .register() on the fixed inference-table destinations or an explicitly-named destination class, never through an arbitrary subclass's inherited __proto__

An explicitly-named destination class is arbitrary — it is whatever the user puts in the dict. Measured: code={IPv4: TransType.ICMP} is accepted, and since '__proto__' in IPv4.__dict__ is False, it writes through IPv4's inherited table, landing in Internet.__proto__ and becoming visible from IPv6. That is the ownership defect exactly, now reachable from a class keyword rather than only from a .register() call. The underlying behaviour is pre-existing and nothing regresses — but the exposure is wider than the sentence claims, which makes the ownership fix slightly more urgent rather than less. Worth noting too, for the owner's open design question: if writing through an inherited registry is later made to raise, code={IPv4: …} — legal today — starts raising, so that decision now has a second caller to consider.

Two minor, non-blocking observations in the detailed comment: an empty code= ({}, [], (), set()) is a silent no-op, which sits oddly beside a design whose whole point is that omission is explicit and inference refuses rather than doing nothing quietly; and isinstance(code, dict) should probably be collections.abc.Mapping, since a non-dict Mapping currently falls into the iterable branch and reports a nonsensical message.

Green everywhere I ran it: new tests 19 passed (exit 0), the five cited test files 50 passed / 94 subtests (exit 0, matching your numbers exactly), tests/project/test_public_api.py 10 passed / 432 subtests (exit 0), changelog drift exit 0. And the full tier reproduces your numbers exactly: 1279 passed, 17 skipped, 2850 subtests passed in 1189.89s, exit code 0 read from a file. That count is also self-consistent — I measured main at 1260 across this session, plus this PR's 19 new tests = 1279.

[JarryShaw]

Detailed cross-review — #570 @ 3f8820ade

Reviewer: Opus 5, per the standing rule that an agent-raised PR gets a cross-review from a different model than the one that wrote it. All measurements with PYTHONSAFEPATH=1 and PYTHONPATH pinned to my worktree, asserting pcapkit.__file__ resolves inside it before importing anything else. Exit codes read from files. Class-creation behaviour measured only through real class statements, never type(name, bases, ns, **kw) — the latter consumes metaclass= differently and has misled an earlier reviewer on this exact question.

Shape verified: one commit, git merge-base --is-ancestor 8cfd6ab01 HEAD true, 9 files, +676/−6.

---

1. Opt-in gating

Declared a ProtocolBase subclass with no code= and diffed all seven dispatch tables (Link, Internet, TCP, UDP, SCTP, Frame, PCAPNG) around the class statement:

declared without code= -> OK
all seven __proto__ tables unchanged? True

Omission registers nowhere, as designed.

2. Inference — the key-type table, re-derived

Rather than trust the table in the body, I declared one subclass per enum type and recorded which tables changed:

key type	landed in	expected
EtherType	['Link']	['Link']	OK
TransType	['Internet']	['Internet']	OK
PayloadProtocolIdentifier	['SCTP']	['SCTP']	OK
LinkType	['Frame', 'PCAPNG']	['Frame', 'PCAPNG']	OK

The LinkType fan-out hits both and only both. Nothing leaks into TCP/UDP.

I agree with the framing that LinkType → two classes is determinism, not ambiguity: register_linktype already fans out to both by hand, so the table encodes existing behaviour rather than inventing a policy.

3. Refusal — the property this design exists for

The failure mode worth preventing is silently picking a registry, because that dispatches to the wrong protocol. So I tried to make it fall through six ways. It refuses every time, and — importantly — mutates nothing on the way out:

code=	outcome	registries touched
8080 (a bare port)	RegistryError: raw key 8080 has no destination registry it can infer; pass an explicit destination, e.g. code={TCP: 8080}	False
an AppType member	RegistryError: no destination registry is known for enum type 'AppType' (key …)	False
'tcp'	RegistryError: code must be an enum member, a {destination: key} mapping, or an iterable thereof, not 'tcp'	False
b'tcp'	same	False
1.5	RegistryError: raw key 1.5 …	False
[None]	RegistryError: raw key None …	False

Two details I want to credit, because both are easy to get wrong:

• The str/bytes guard sits before the general Iterable branch, so a string is rejected rather than silently iterated as a sequence of characters.
• The enum branch sits before the iterable branch, so an IntEnum-style member is treated as an enum rather than as an int. And AppType being absent from the table is correct rather than an omission: its members span both TCP and UDP, so it genuinely cannot infer.

Q12 also holds: code={Internet: <TransType member>} — an explicit destination for an inferable key — is accepted and lands correctly. And an unrecognised keyword raises UnsupportedCall: Probe: unexpected class keyword(s): nonsense.

4. Backward compatibility — the load-bearing safety claim, re-derived

I did not take this one on trust. I dumped, in a canonical sorted form, the Protocol and ProtocolBase transitive descendant counts plus every entry of all seven __proto__ tables (rendered as KeyType:key -> ClassName, with ModuleDescriptor flattened to module.name), with pcapkit.all imported so every registry is fully populated. Run in separate fresh interpreters on main and on this head:

Protocol direct+transitive descendants:      0
ProtocolBase direct+transitive descendants:  43
… 38 registry entries across 7 tables …
SHA256: 8f4ef56585cf395e8409a70d44a2cf02b2b9f2f2deeb1e79a63a7341403e832f

Identical hash on both trees, and diff of the two dumps is empty. So the 0/43 counts and the byte-identical tables are confirmed independently.

5. The Python 3.10 keyword hazard

type(ProtocolBase) is ProtocolMeta, MRO ['ProtocolMeta', 'ABCMeta', 'type', 'object'] — so ABCMeta.__new__ really is in the path and the part (a) hazard is real here too. Measured through real class statements on both interpreters:

class keyword	Python 3.10.21	Python 3.14.7
code=	accepted, lands in Internet.__proto__	accepted, lands correctly
name=	TypeError: ABCMeta.__new__() got multiple values for argument 'name'	UnsupportedCall: … unexpected class keyword(s): name
mcls=	TypeError: … 'mcls'	UnsupportedCall
bases=	TypeError: … 'bases'	UnsupportedCall
namespace=	TypeError: … 'namespace'	UnsupportedCall

code is not one of the four colliding names, so it is safe — and the test's

expected = UnsupportedCall if sys.version_info >= (3, 11) else TypeError

is exactly right. I verified both branches of that condition on real interpreters rather than only the one this host defaults to. Worth stating plainly why the split exists: on 3.10 the metaclass raises before __init_subclass__ runs at all, so the PR's UnsupportedCall guard cannot catch those four names there, and the test correctly does not pretend otherwise. (The test covers name= only; mcls/bases/namespace behave identically, which I checked — not worth adding.)

6. Fails-without-the-fix

Reverted all five changed source files to main (pcapkit/protocols/protocol.py, pcapkit/foundation/registry/protocols.py, pcapkit/all.py, pcapkit/foundation/__init__.py, pcapkit/foundation/registry/__init__.py), keeping the new tests, then restored from byte-identical copies and proved git status clean and git diff HEAD empty.

18 failed, 1 passed
exit code (from file): 1

and with the fix, 19 passed, exit 0. The single test that passes both ways is test_backward_compatible_dispatch_tables_are_unaffected, which is exactly what the body says it is. 18/19 confirmed.

7. The ownership interaction — your specific question

Answer: orthogonal; not harder; no hasattr trap; can merge first. But the Scope note overstates the separation.

Why it is not harder: register_protocol_code's entire write path is

for destination, key in _iter_code_targets(code):
    destination.register(key, protocol)

It performs no registry resolution of its own — no hasattr, no getattr('__proto__'), no walking the MRO. It delegates to the existing register classmethods. So when the ownership guard lands in those (tested with '__proto__' in cls.__dict__, not hasattr, per the thread's trap), this mechanism picks it up for free. The diff introduces hasattr only twice, both in tests asserting __schema__/__data__ exist. Nothing here builds the trap in.

Why the Scope note is nevertheless wrong. It says .register() is called on "the fixed inference-table destinations or an explicitly-named destination class, never through an arbitrary subclass's inherited __proto__". The explicitly-named destination is user input, so it can be any class:

'__proto__' in Internet.__dict__ = True      # Internet owns its table
'__proto__' in IPv4.__dict__     = False     # IPv4 does not

code={IPv4: TransType.ICMP}  -> ACCEPTED
  landed in Internet.__proto__?  True
  visible from IPv6.__proto__?   True

So the dict form reaches the ownership defect directly, and from a class keyword rather than from an explicit .register() call. Nothing regresses — IPv4.register(...) already did this — but the surface is wider than claimed, which argues for the ownership fix sooner rather than later. And it gives the owner's open design question a second caller: if writing through an inherited table is made to raise, then code={IPv4: …}, legal today, begins raising too. Please correct that sentence; the conclusion ("doesn't affect this mechanism's correctness") is fine for the inference path, just not for the explicit-mapping path.

I did not treat the missing ownership fix as grounds for NEEDS CHANGES. Deferring it is reasonable and the PR flagged it rather than hiding it.

8. Minor, non-blocking

(a) An empty code= is a silent no-op.

code={}     -> accepted, registries changed: False
code=[]     -> accepted, registries changed: False
code=()     -> accepted, registries changed: False
code=set()  -> accepted, registries changed: False

None already means "decline registration", so an empty container is far more likely a mistake — a list built programmatically that came out empty, say. It sits oddly next to a design whose stated principle is that inference refuses rather than quietly doing nothing. Raising, or at least a RegistryWarning, would fit the surrounding philosophy better.

(b) isinstance(code, dict) should probably be collections.abc.Mapping. A Mapping that is not a dict falls through to the iterable branch, which iterates its keys — the destination classes — and produces advice that cannot be followed:

RegistryError: raw key <class '...Internet'> has no destination registry it can infer;
              pass an explicit destination, e.g. code={TCP: <class '...Internet'>}

OrderedDict works, being a dict subclass. collections.abc is already imported in the module, so this is a one-word change. Low likelihood, confusing when hit.

9. Other checks

• tests/protocols/test_protocol_code_registration_unit.py → exit 0, 19 passed.
• test_dispatch_registry_unit.py, test_registry_runtime.py, test_protocol_base_unit.py, schema/test_schema_unit.py, schema/test_enum_schema_registry_unit.py → exit 0, 50 passed, 94 subtests — matching the body's numbers exactly.
• tests/project/test_public_api.py → exit 0, 10 passed, 432 subtests. Relevant because register_protocol_code is added to three __all__ lists and this file holds pcapkit.foundation.registry to the stricter "every public attribute is exported" rule.
• Full tier pytest tests (no coverage, per the host memory constraint — coverage run -m pytest tests reached 36.9 GB RSS from a sibling worktree earlier today and exhausted this machine) → exit 0 from a file, 1279 passed, 17 skipped, 2850 subtests passed in 1189.89s. Matches the body exactly, and is self-consistent: main measured at 1260 elsewhere in this session, plus 19 new tests.
• python util/changelog_md.py --check → exit 0, in step with 1.5.0.rst. The entry's content is accurate against every measurement above, including the LinkType double destination and the raw-int requirement.
• Protocol.__init_subclass__ forwards as super().__init_subclass__(schema, data, code, *args, **kwargs), which lines up positionally with ProtocolBase.__init_subclass__(cls, /, schema=None, data=None, code=None, *args, **kwargs). Correct, and the unrecognised-keyword guard lives in the base so both paths get it.

Could not verify

• CI. Reached deliberately on local evidence only, per my brief; I make no claim about its tally.
• The #548 claim that the fix becomes a one-declaration code=[TransType.L2TP, {UDP: 1701}]. The shape is consistent with everything I measured about the mixed-iterable form, but I did not actually declare an L2TP subclass that way and drive a capture through it.
• Part (c) (the ~76 *Base as <Public> alias imports) and its claimed independence from this PR — out of scope and untested by me.
• aenum vs enum coverage. The guard is isinstance(code, (enum.Enum, aenum.Enum)). Every const enum I exercised was caught, but I did not enumerate every enum class in pcapkit.const to confirm none is neither.
• Interpreters other than CPython 3.14.7 and 3.10.21. Those two I did measure, which is what the version-pinned test needed.