fix(registry): add register_protocol's identity guard to nine sibling registrars

[JarryShaw]

• Searched for similar pull requests
• Followed the coding style (make pylint, make mypy, make isort)
• make test passes, and a test case covers the change — ran the targeted suite only (see below), not the full make test
• Added a changelog entry under docs/source/changelog/ and regenerated CHANGELOG.md, if the change is user-visible — N/A, changelog centralised in docs(changelog): shared 1.5.0 changelog — long-lived, merges last (#610, #616, #617, #618, #620) #657

What is the purpose of your pull request?

• fix — corrects a defect

---

Description of your pull request and other information

Nine code-keyed registrars warned on presence alone, so re-registering the exact same class read as "overwriting X with X" — worse than #710/#711, since nothing distinguished that no-op from a real collision. Added register_protocol's identity guard (incumbent is not None and incumbent is not X) to all nine; message text unchanged.

File	Line	Method
protocol.py	801	ProtocolBase.register
internet.py	166	Internet.register
link.py	146	Link.register
transport.py	117	Transport.register
sctp.py	630	SCTP.register
frame.py	153	Frame.register
pcapng.py	888	PCAPNG.register
schema.py	1131	EnumSchema.__init_subclass__ (reachable via ordinary subclassing, not symmetry-only: a repeated or aliased member in code=[...] hits the same key twice with cls on both sides)
schema.py	1186	EnumSchema.register

Step 2 (#711's id() disambiguation) — not applied. #711's own cross-review found the id() suffix defeats __warningregistry__ dedup, with a nondeterministic delivered-warning count under GC pressure (199, 191, then 3 of 200 in one measurement). Once step 1 removes the dominant "X with X" cause, the residual case — two different classes sharing a repr — is mostly a dynamic-factory/test-class shape, not a real-usage concern. Paying that cost nine more times isn't worth it; the identity guard alone is the fix.

Falsification: one same-object no-op test per site (9 total), each shown failing against the unfixed guard with the literal "overwriting X with X" text, passing after. Also fixed 6 pre-existing tests that used a same-object re-registration as their "overwrite" case, incl. test_sibling_registries_still_warn_on_an_identical_re_registration, which pinned the old behaviour by name. Added a tenth test, test_a_repeated_code_in_the_declaration_list_stays_quiet, covering schema.py's guard through ordinary class X(Base, code=[A, A]) syntax and an enum-alias pair — the prior per-site test only called __init_subclass__() directly.

Coverage (9 files, targeted suite): 3294→3301 stmts, miss and BrPart unchanged (180/97) — every new statement/branch fully exercised. pylint/mypy message counts unchanged in content (364/21) after retargeting two # type: ignore comments mypy flagged as stale.

Could not verify: full make test; CI (not run).

Fixes #718.

[JarryShaw]

❌ NEEDS CHANGES @ d12c3b441 — EnumSchema.__init_subclass__'s guard is reachable through ordinary code=[X, X] / enum-alias syntax (measured: warns pre-fix, silent post-fix), so schema.py:1125-1130, the test_declaring_the_same_class_twice_stays_quiet docstring and the PR table all assert the opposite of what it does — correct all three and test that path.

[JarryShaw]

❌ NEEDS CHANGES @ d12c3b441 — EnumSchema.__init_subclass__'s guard is reachable through ordinary code=[X, X] / enum-alias syntax, so schema.py:1125-1130, the test_declaring_the_same_class_twice_stays_quiet docstring and the PR table all assert the opposite of what it does — correct all three and test that path.

Independent cross-review, Opus (author: Sonnet). Verified on d12c3b441 and on a clean merge of origin/main.

Blocker. code= is an Iterable and the loop at schema.py:1124 visits every element, so a repeated member reaches the same key twice with cls on both sides. With that hunk reverted:

code=[Code.one, Code.one]                          -> schema 1 already registered, overwriting <class 'abc.Dup'> with <class 'abc.Dup'>
code=[Code.one, Code.uno]  (Code.uno is Code.one)  -> same
code=[Code.one, Code.two]  (control)               -> silent

Post-fix all three are silent, so the guard is live rather than symmetry-only. Not a contrived shape either: #721 just put that list form into production (R1CounterParameter(Parameter, code=[R1_Counter, R1_COUNTER]) — 128/129, distinct, so it does not fire), and 4 pcapkit.const enums carry real aliases, reg.linktype.LinkType (I2C_LINUX) and esp.cipher.Cipher (30) among them. Please cover it too: the only test for this site calls __init_subclass__() directly, which its own docstring calls "the only way" — the claim being corrected.

Also. pcapkit/protocols/schema/misc/pcapng.py:725-733 (Option.register, untouched here) still defends its presence-only guard with "which is what the seven sibling register methods on ProtocolBase and friends already assume". This PR falsifies that premise and leaves the prose asserting it; it is a 10th sibling in the family, still presence-only.

claim	verdict	what I derived independently
9 sites, none pre-guarded	✅	9 presence checks removed / 9 incumbent is not None and incumbent is not X added, across 8 files
same guard as register_protocol	✅	condition byte-identical to protocols.py:221; #711's id() half deferred, as #718 itself asks
identity, not equality	✅	is not throughout, and correct — ProtocolBase.__eq__ (protocol.py:1325) is name-based against str, so == could hide the very #710 collision the warning exists for
schema.py opt-out intact	✅	if code is not None: unchanged; no-code= and explicit code=None → 0 warnings, registry unchanged. dict.get never calls __missing__, so #555's non-recording miss still holds
tests fail without the fix	✅	revert transport.py → Expected 'warn' to not have been called. Called 1 times. … "port 80 already registered, overwriting <class '…Raw'> with <class '…Raw'>" (1 failed/10 passed). revert schema.py → both new tests fail on "schema 1 already registered, overwriting <…Twice'> with <…Twice'>" (2 failed/10 passed)

Reach is narrower than the body implies. Seeded registries hold ModuleDescriptors, so re-registering a built-in under its own built-in code still warns (overwriting ModuleDescriptor(module='pcapkit.protocols.link.arp', name='ARP') with <class …ARP>) — the guard only helps once the incumbent is already a class. A walk_packages sweep of all of pcapkit emits 0 RegistryWarnings pre- and post-fix: latent, not user-visible.

breaking label — your call, not a blocker. Against: the registry write is identical on both paths and the package's own imports are unaffected. For: 6 pre-existing tests had to change, one pinning the old behaviour by name. I read it as not breaking, since the guard only ever fired where nothing was displaced.

Runs (3.14.7, pcapkit proven to load from the worktree; fixtures generated, 0 skipped). Branch, 9 test files: 203 passed, 1930 subtests, exit 0. Merged tree — origin/main is now 1aae1da30, not daa953d1b — merge exit 0 with 0 conflicts; those 9 files plus the 3 main just touched: 250 passed, 2468 subtests, exit 0. mypy: 21 errors in the 8 touched files (matching the body), none on a changed line, and no unused-ignore at schema.py:1131/1186, so the retargeted codes are right under warn_unused_ignores = True.

#726 vs #728: disjoint library files, and the shared test_pcapng_unit.py hunks sit ~2700 lines apart, so no conflict either way. I would land #728 first — #726 needs another revision regardless, so its mandatory rebase absorbs #728 rather than the reverse, and the Option.register fix above would pull #726 into schema/misc/pcapng.py, which #728 rewrites.

❌ NEEDS CHANGES @ d12c3b441 — correct the three reachability claims, add a code=[X, X]/alias test, and update schema/misc/pcapng.py:725-733's now-false cross-reference.

[JarryShaw]

❌ NEEDS CHANGES @ 6634e2283 — correct schema/misc/pcapng.py:725-734's two now-false sentences (my d12c3b441 third demand, unaddressed; Option.register is a 10th sibling I measured still self-displacing) and the body's coverage pair (116/80) → (180/97).

[JarryShaw]

❌ NEEDS CHANGES @ 6634e2283 — correct schema/misc/pcapng.py:725-734's two now-false sentences, and the body's (116/80) → (180/97).

Independent cross-review, Opus (author: Sonnet). Supersedes my d12c3b441 verdict, which made three demands: two are done. The reachability corrections are accurate rather than merely softened — schema.py:1125-1132, the test_declaring_the_same_class_twice_stays_quiet docstring and the body table now each state positively that a repeated or aliased member in code=[...] reaches the guard through ordinary syntax. The third demand is untouched.

Blocker 1 — Option.register (pcapkit/protocols/schema/misc/pcapng.py:725-734), still presence-only, prose now false in two ways.

• "__init_subclass__ passes each code exactly once per subclass" — disproved. Option.__init_subclass__:700-702 loops an iterable code calling Option.register per element, the same shape as the site you just fixed. Measured at this head in namespace dsb on a code absent from it: control code=a → 0 warnings; code=[b, b] → 1 warning, option already registered in namespace(s) 'dsb', overwriting with <class '…RepOption'> — self-displacement, the exact text RegistryWarning: nine sibling registrars share #710's repr collision, and guard on presence rather than difference #718 exists to remove.
• "which is what the seven sibling register methods on ProtocolBase and friends already assume" — this PR removes that assumption, so the sentence cites the nine methods you just changed as authority for a premise they no longer hold. Left as-is, the package asserts the opposite of this PR's own new comment two files away.

Correcting those two sentences is enough to clear this; the guard itself is fair follow-up, since #728 rewrites that file.

Blocker 2 — the body's coverage pair does not reproduce. Under the body's own scope (nine modified test files as the pytest selection, eight modified library files as the coverage scope), both sides measured: 4391dc77b → 3294/180/1292/97, 6634e2283 → 3301/180/1292/97. So "3294→3301 stmts" is exact and "miss and BrPart unchanged" is exactly true — but the pair is 180/97. (116/80) reproduced under no scope tried. "Every new statement/branch fully exercised" is independently confirmed: +7 statements, 0 new branches, none missed.

claim	verdict	what I derived independently
three assertions corrected, not softened	✅	all three affirmative; the direct-call test now cross-references the ordinary-syntax one
new test fails without the library hunk	✅	reverted schema.py to 4391dc77b → 3 failed / 10 passed, exit 1 (you reported 2/11); RepeatedMember and AliasPair each warn displacing themselves; restored → 13/13
nine registrars, all previously defective	✅	exactly 9 presence-only guards at 4391dc77b, exactly 9 identity guards at head, condition matching protocols.py:221; registry write stays outside the guard at all nine, so nothing is dropped
is not ==	✅ code, ❌ my stated reason	my d12c3b441 rationale was wrong. __eq__ is a classmethod, so Class == Class resolves on the metaclass, and ProtocolMeta/SchemaMeta/EnumMeta all inherit object.__eq__ — two distinct classes with identical reprs compare == False. == would not have hidden #710 at these sites. is is still correct, just for a different reason.
schema.py opt-out survives	✅	no code= and explicit code=None → 0 RegistryWarnings, registry unchanged; _EnumRegistry overrides only __missing__, so .get() really returns None
breaking	—	your call already recorded as no; not re-litigated

Also measured. Merged into current main (bf57b4542) with --no-ff: 0 conflicts, 207 passed, exit 0; the one auto-merged file is ~3,600 lines apart from your hunks. Branch-only: 204 passed, 1930 subtests, exit 0, 0 skips — matching the amended message, so the old "249 passed, 1 skipped" is gone. No test was weakened: 10 added, 0 removed (one rename), and real-collision coverage grew from 3 registrars to 8. After #727, CI's test job still runs all 11 new tests (verified fixture-less: 206 passed, 1 pre-existing skip). [index]→[arg-type] is the right retarget — 0 unused-ignore in schema.py under warn_unused_ignores. One body-accuracy note: reach is narrower than the body implies — all 7 Link.__proto__ entries are seeded ModuleDescriptors, and registering the very class one names still warns; the guard only helps once the incumbent is already a class.

❌ NEEDS CHANGES @ 6634e2283 — the two Option.register sentences, and (116/80) → (180/97).

[JarryShaw]

Addressed both remaining items from the last review:

• Option.register's docstring (pcapkit/protocols/schema/misc/pcapng.py:717-735): corrected the two now-false sentences. It no longer claims __init_subclass__ passes each code exactly once per subclass (it loops over a code list, so a repeated/aliased member reaches this method twice with the same class, same as the failure just fixed elsewhere), and no longer cites the seven ProtocolBase-family siblings as still assuming that -- they were the ones just fixed away from it. The "seven" count itself is correct; only the claim about their current behavior was wrong. Also tightened the #681 cross-reference in the first paragraph, which called it "the same guard" while the next paragraph says otherwise.
• PR body coverage figure corrected: (116/80) -> (180/97).

Amended onto the existing commit, force-pushed. New head: 22b57397e.