[JarryShaw]

Please follow the guide below

• 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
• N/A, 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?

• feat — adds a feature

Description of your pull request and other information

Closes #2448. Implements IGMPv1, IGMPv2 and IGMPv3 as one Internet subclass (pcapkit.protocols.internet.igmp). Depends on #2444.

• One 8-octet GroupBody covers IGMPv1/v2 messages and the IGMPv3 Query; a Query of at least 12 octets adds a QueryExtension (source list). A 9-11 octet Query is refused and falls back to Raw.
• ReportBody covers the IGMPv3 Membership Report (0x22), with all six RFC 3376 §4.2.12 record types.
• The RFC 3376 §4.1.1 floating-point Max Resp Code applies only to an IGMPv3 Query; an 8-octet message's code is literal. 0x8c decodes to 22.4 s.
• IGMP.length is never below the 8-octet fixed header, so a cut header falls back to Raw instead of zero-filled fields.
• IGMP is wired into the Internet dispatch table and the census tests that enumerate protocols and constants.

Testing: unit tests in tests/protocols/internet/test_igmp_unit.py; coverage run -m pytest tests/protocols/internet/test_igmp_unit.py tests/corekit/test_schema_roundtrip_unit.py passes.

[JarryShaw]

This implements IGMPv1/v2/v3 all in one IGMP class — but PR #2444 is currently being redesigned (per the owner's ruling) to split IGMPv1/IGMPv2 into separate classes under a shared dispatcher, mirroring this codebase's HTTP/HTTPv1/HTTPv2 pattern. That redesign is still in progress. These two can't both land as-is: this PR's v3 group-record-list parsing logic is valuable and will carry over, but it needs to become a separate IGMPv3 class fitting the new split architecture, not merged into one class with v1/v2.

Also: this PR touches CHANGELOG.md and docs/source/changelog/1.6.0.rst directly — per the standing practice from #2449, changelog entries are now added centrally rather than per-PR; that part will need dropping regardless of the architecture question.

Holding this for re-architecture once #2444's split lands. Unblocks when #2444 merges.

[github-actions]

Coverage: 94.46% (unit tier, Python 3.15, 66317ed8a, Unit Tests run success)

Package	Statements	Missed	Branches	Partial	Cover
pcapkit (top level)	104	4	20	2	95.16%
pcapkit/const	19760	133	2646	120	98.85%
pcapkit/corekit	2605	74	818	37	96.52%
pcapkit/dumpkit	281	3	100	3	98.43%
pcapkit/foundation	3183	27	1110	31	98.65%
pcapkit/interface	138	4	52	3	96.32%
pcapkit/protocols	18668	204	4906	189	98.32%
pcapkit/toolkit	924	36	334	10	95.55%
pcapkit/utilities	502	7	144	5	98.14%
pcapkit/vendor	4532	1897	1038	170	54.88%

Per-file detail: the coverage-html artifact of this run.

[JarryShaw]

CI confirms the conflict already flagged: every Python version fails. Real defects, not flakes —

• internet.igmp:IGMP does not round-trip / IGMP/patterned does not round-trip: UNBUILT: no minimal instance packs: ProtocolError: IGMP: unsupported message type: <Type.Reserved_0: 0>
• internet.igmp:GroupBody does not round-trip: REPARSE: KeyError: 'type'
• test_public_api.py: IGMP_RecordType/IGMP_Type leak into the public surface unexpectedly

Holding this rather than dispatching a fix: #2444 is mid-redesign into split IGMPv1/IGMPv2 classes behind an IGMP dispatcher, and this PR's monolithic one-class-does-everything shape would need rebuilding against that the moment it lands, not patched in place. Labeling #2448 blocked on #2444.

[JarryShaw]

NEEDS CHANGES (Sonnet, 28fca7a16). IGMPv3 round-trips and the public API are fixed; the hold on #2444 is respected.

• Regression: IGMP.length counts consumed octets, so the short-header guard (protocol.py ~L2475) never fires. A 1-7 octet IGMP payload stays IGMP with zero-filled fields and a SchemaWarning. Make length the declared size (or reject under 8 octets) and add a test.
• Timed prose: test_igmp_unit.py docstrings open "A cross-review regression (feat(internet): implement IGMPv3 (group-record list) #2448)" (six places) and say "pre-feat(internet): implement IGMPv3 (group-record list) #2448 code", "this follow-up", "before the fix". Remove.
• IGMP class docstring says "more than 8 octets"; RFC 3376 §7.1 and the code say at least 12.
• PR body: 38 lines, limit 15. Drop timed context; tick the changelog box as "N/A, centralised in docs(changelog): shared 1.5.0 changelog — long-lived, merges last (#610, #616, #617, #618, #620) #657"; fix "32 unit tests" (33); "supersedes feat(internet): add IGMPv1/IGMPv2 as concrete classes over an abstract base #2444" contradicts the hold.

[JarryShaw]

NEEDS CHANGES (Sonnet, 40b839c4e). The 8-octet floor and the fallback to Raw are verified; the 8- and 12-octet round trips are byte-exact.

• igmp.py ~L296, ~L450 and data/internet/igmp.py ~L67: the RFC 3376 float Max Resp Code is applied to 8-octet IGMPv2 messages. Scope it to the Query extension (12+ octets). An 8-octet Query with code 200 currently reads 3072.
• A 10-octet Query stays IGMP with a Raw tail; feat(internet): add IGMPv1/IGMPv2 as concrete classes over an abstract base #2444 gives Raw. make() accepts max_resp_time 256, True and 1.5, which feat(internet): add IGMPv1/IGMPv2 as concrete classes over an abstract base #2444 rejects.
• Timed prose: test_igmp_unit.py ~L119-122 and ~L471-472; PR body "waits on feat(internet): add IGMPv1/IGMPv2 as concrete classes over an abstract base #2444 ... then rebased" becomes "Depends on feat(internet): add IGMPv1/IGMPv2 as concrete classes over an abstract base #2444".

[JarryShaw]

Good-to-go CI watch caught a red check on e0841ec71: Python 3.15's merge-ref test failed EXPECTED_CRAWLERS_WITH_CONSTANT_MISS_LINES: 37 != 36. Old head e0841ec71, new head 1afc892ce.

Root cause: this branch's own pin (36) was self-consistent on its own base, but main has since gained the ARPHRD crawler (#2497), and this branch's own igmp/record_type.py crawler adds a second -- so the correct value once merged with current main is 36 (main's original baseline, pre-ARPHRD) + 1 (ARPHRD) + 1 (record_type.py) = 38... correction, verified directly: confirmed 36 actual on the standalone branch alone, 37 actual on the merged tree (ARPHRD +1 over the pre-ARPHRD baseline this branch forked from), so 37 is the correct pinned value post-rebase.

Patch outside the pin is unchanged -- rebased cleanly onto origin/main (57ff1c8c5) with no conflicts. Verified: tests/vendor/, tests/const/ pass clean (580 tests, 26693 subtests).

[JarryShaw]

Decision needed: how to reconcile IGMPv3's design with main's already-merged IGMP v1/v2 (#2444).

Context: this branch was built before #2444 existed, as a single concrete IGMP class parsing v1/v2/v3 uniformly — direct IGMP(wire, length) is the entry point, and its whole (already-approved, GOOD TO GO) test suite is written against that. Main's merged #2444 instead made IGMP an abstract base (version is @abc.abstractmethod, enforced by test_base_is_abstract) with separate concrete IGMPv1/IGMPv2 sibling classes dispatched via _classify. These are incompatible meanings for the same class at the same import path -- not a mechanical rebase conflict.

One thing favors a path forward: main's own _classify already carries a comment anticipating this -- "Longer queries are IGMPv3 and other lengths are ignored" -- so the abstract-base extension point was left open for a third version.

Options:

1. Add a concrete IGMPv3 sibling class alongside IGMPv1/IGMPv2, matching main's established pattern, and rewrite this branch's test suite to address all three as separate concrete classes (mirroring how main's own test_igmp_unit.py already tests v1/v2). Keeps main's reviewed architecture intact; costs a substantive rewrite of this branch's already-approved tests.
2. Revert main's abstract-base design back to a single concrete IGMP class covering all versions, reopening a decision feat(internet): add IGMPv1/IGMPv2 as concrete classes over an abstract base #2444's reviewers already signed off on.

Recommendation: option 1. It extends an already-merged, already-reviewed pattern rather than unwinding a settled decision on main, and main's own code comment shows the extension point was anticipated. The cost is real (dozens of tests to rewrite) but contained to this one unmerged branch.

[JarryShaw]

Option 1 executed: IGMPv3 added as a concrete sibling, test suite rewritten for three classes.

Old head 1afc892ce, new head 66317ed8a (4 commits: the architecture, two small fixups already in the branch history, plus one new verification-fixes commit).

What changed

The branch's own igmp.py (a single concrete IGMP class parsing v1/v2/v3 uniformly) is gone. In its place, following #2444's already-merged pattern exactly:

• pcapkit/protocols/internet/igmp.py (abstract base) — version widened to Literal[1, 2, 3], id() now ('IGMP', 'IGMPv1', 'IGMPv2', 'IGMPv3'), _classify/_dispatch extended: an 8-octet Membership Query is v1/v2 as before, 12-or-more octets is v3's extended Query, the IGMPv3 Membership Report (0x22) is always v3, and 9-11 octets stays unclassifiable (falls back to Raw, unchanged from #2439's ruling).
• igmpv1.py/igmpv2.py — untouched, copied verbatim from origin/main.
• igmpv3.py (new, 502 lines) — the third sibling. Unlike v1/v2, which reuse the base's flat 8-octet schema/data classes as-is, IGMPv3's wire format differs materially by message (a Query's source-list extension, a Report's group-record list), so it carries its own schema and data module — schema/internet/igmpv3.py (331 lines) and data/internet/igmpv3.py (133 lines) — the same way HTTPv1/HTTPv2 each carry their own under the shared HTTP base. This is an established mechanism (Protocol.__init_subclass__'s schema=/data= kwargs are per-subclass), not a new one.
• The floating-point Max Resp Code/QQIC decode (_decode_resp_code/_encode_resp_code), the group-record list, the extended-Query flags/QQIC/source-list, and the 0x22 message type all moved into igmpv3.py from the old monolithic igmp.py, logic unchanged — this was a relocation, not a rewrite of the wire-format handling itself.

id()/alias reasoning for the new sibling

Per the task's own instruction to verify independently rather than copy: RFC 3376's title is "Internet Group Management Protocol, Version 3 (IGMPv3)" — it carries its own version, the same as RFC 2236 for IGMPv2 (which also keeps its version in id()/alias), unlike RFC 1112's plain IGMP for v1 (the subject of the still-open #2506). So IGMPv3.id() is ('IGMP', 'IGMPv3') and alias is 'IGMPv3' (inherited from the base's existing f'IGMPv{self.version}' formula — no override needed). #2506 is not yet merged, so the base's id() currently mirrors the pre-#2506 pattern (family-inclusive for every version); this PR adds IGMPv3 as a third member of that same shape rather than anticipating #2506's narrow-to-self change, which is orthogonal and unmerged.

Rebase surprise

The branch's actual merge-base with origin/main was 57ff1c8c5 — the commit immediately before #2444 merged — not the later point the branch's own history implied. So this was a real git rebase onto origin/main, picking up #2444 (IGMP v1/v2 split), #2451 (DTLS reassembly), and #2505 (ARPHRD docs) along the way, not just an IGMP-only patch. All conflicts were in IGMP-adjacent files (const/vendor record_type registration, top-level __all__ lists, pep.rst wording, one stray leftover <<<<<<< HEAD marker from the branch's own earlier troubled history) — nothing in the DTLS/ARPHRD work conflicted.

Test suite rewrite

tests/protocols/internet/test_igmp_unit.py rewritten in full (1133 lines), mirroring origin/main's own structure for v1/v2 (IGMPRoundTripTests, IGMPReadTests, IGMPVersionedClassTests, IGMPUnitTests, IGMPDispatchTests) and extending each to the third class, plus three v3-specific classes carried over from this branch's already-approved suite (IGMPv3MaxRespCodeTests, IGMPv3QueryTests, IGMPv3ReportTests), adapted to address igmpv3.IGMP directly instead of the old monolithic class. 66 test methods / 231 subtests.

Const/vendor carry-over: const/igmp/record_type.py and vendor/igmp/record_type.py are byte-identical to the branch's pre-rebase versions (confirmed by diff) — the record-type registry is architecture-independent. Census pins (ints, enums, module counts, all_files, generated, the vendor-crawler-missing-body count) were re-measured against current origin/main, not the branch's stale pre-#2444 baseline: every one is origin/main's current value +1, since adding record_type.py is the only new const/vendor module regardless of the IGMP architecture.

Verification

End-to-end, real packet bytes through IPv4 dispatch, not just imports:

v1-query  -> pcapkit.protocols.internet.igmpv1 IGMPv1 (type 17)
v2-report -> pcapkit.protocols.internet.igmpv2 IGMPv2 (type 22)
v3-query  -> pcapkit.protocols.internet.igmpv3 IGMPv3, sources=(10.0.0.1, 10.0.0.2), qrv=2, s_flag=True
v3-report -> pcapkit.protocols.internet.igmpv3 IGMPv3, 1 group record, 224.0.1.1

Test batteries, all clean:

Suite	Result
tests/protocols/internet/test_igmp_unit.py	66 passed, 231 subtests
tests/const/ + tests/vendor/ + test_dispatch_registry_unit.py	588 passed, 26811 subtests
tests/corekit/test_schema_roundtrip_unit.py + tests/project/	2137 passed, 80 skipped (pre-existing/unrelated), 0 failed, 11528 subtests

examples/generators/make_samples.py runs clean (no new failures).

Bugs the full-suite pass caught and fixed (worth flagging explicitly)

1. The rebase's auto-merge of the top-level pcapkit/protocols/data/__init__.py and .../schema/__init__.py __all__ lists resolved without a conflict marker, but left a duplicate 'IGMP' entry plus the old single-class design's stale names (IGMP_GroupAddressMessage, IGMP_GroupBody, etc.) that no longer exist — caught by test_public_api.py's star-import and no-duplicate-entries sweeps, not by anything IGMP-specific.
2. isort wanted different import wrapping in the new igmpv3.py under the project's actual -l100 -ppcapkit flags (my first pass used plain isort with its default shorter line length).
3. The new _classify literal 12 (IGMPv3's minimum extended-Query length) coincidentally equals AH_HEADER_LEN/TCP_DATA_OFFSET_OCTET/GENERIC_BLOCK_MINIMUM; added an UNRELATED entry to test_rfc_constants_one_home_1739_unit.py explaining the coincidence.
4. One of my own new tests (test_refusals_name_the_version_refused) used an 8-octet Report fixture to probe IGMPv3's rejection path; a Report type isn't Query-shaped at all, so IGMPv3's schema selector refuses it with "unsupported message type" before read() ever runs, not the read()-level "is not an IGMPv3 message" message the test expected. Fixed by using an 8-octet Query fixture instead — the one shape all three versions actually share.

Branch is 4 commits ahead of 57ff1c8c5 relative to its old self, now rebased cleanly onto current origin/main (c6b3848d1).

[JarryShaw]

Update on the decision above: since you hadn't weighed in yet and the standing rule here is not to block on an answer, I went ahead with the recommended option (1) -- a concrete IGMPv3 sibling matching main's IGMPv1/IGMPv2 pattern, with its own schema/data classes (the HTTPv1/HTTPv2 precedent, since v3's wire format differs materially from v1/v2's shared shape). New head 66317ed8a. Full details and test counts in the push comment below. Still flagging needs: decision since you haven't actually confirmed this was the right call -- if you'd have preferred option 2 (reopening main's abstract-base design) say so and I'll redo it; otherwise this is ready for a final review pass.

[JarryShaw]

Cross-review (Sonnet) at 66317ed8a: GOOD TO GO on the rework itself. igmpv1.py/igmpv2.py confirmed byte-identical to main's merged versions (0-byte diff). IGMPv3 re-checked against RFC 3376 with real Query/Report packets, correct floating-point Max Resp Code decode. Dispatch through the top-level IGMP class confirmed end to end for all three versions, including wrapped in real IPv4 packets. All 4 claimed bug fixes verified genuine (no duplicate __all__ entries, isort clean, both magic-number exceptions are real false positives, the schema-roundtrip fixture fix is sound). No conflict markers. mergeStateStatus: BLOCKED is just the pending-review gate, not a conflict. One minor note: the review's own tests/project/ run (2132/10621) didn't exactly match the rework's self-reported figures (2137/11528) -- nothing failed either way, likely a difference in which sweeps ran, not a defect.

needs: decision stays on -- the technical work is sound, but you still haven't actually confirmed this was the right architectural call (vs. reopening main's abstract-base design). Ready for your sign-off, then merge.

[JarryShaw]

yes option 1, IGMPv3 gets its own class

[JarryShaw]

Confirmed: IGMPv3 gets its own concrete class (Option 1). The architectural choice is approved; ready to merge.