fix(tcp): set MPTCP.subtype on construction, fix MP_CAPABLE's length/rkey (#566, #567)

[JarryShaw]

This PR was originally branched from #565's head and stacked on it; #565 has since merged,
so this has been rebased onto current main and now targets it directly. #565 gave MPTCP
real kind/length fields, which is the foundation both fixes here build on -- without it,
construction never gets far enough to reach either defect.

Summary

• Fixes MPTCP.subtype is declared only under TYPE_CHECKING, so every subtype built via TCP's convenience constructor raises AttributeError #566: MPTCP.subtype was declared only under typing.TYPE_CHECKING, so it was an
  annotation, never a field, and the only code that ever set it was
  _MPTCP.post_process, which runs only on a real byte-level unpack. TCP's convenience
  constructor (TCP(options=[(Enum_Option.Multipath_TCP, {...})])) builds a schema in memory
  and reads it straight back through _read_mptcp_* with no byte round trip, so every subtype
  but MP_JOIN raised AttributeError: ... has no attribute 'subtype'. Fixed on the
  construction path (TCP._make_mode_mp, the single dispatcher every _make_mptcp_* maker
  returns through) rather than by adding a third real field the way fix(tcp): give MPTCP real kind/length fields, repairing pack and parse #565 did for kind/
  length: unlike those two, subtype is already packed as 4 bits of each subtype's own
  test bitfield, and a second, independent field for the same bits would either double-pack
  them or need a "derive, don't pack" field kind this library's corekit.fields does not have.
  That reasoning is recorded as a comment on MPTCP itself.

• Fixes TCP._make_mptcp_capable writes length 20/32 where RFC 8684 gives 12/20, and now packs wrong bytes rather than crashing #567, three sites, not two -- the issue names only the maker, but two more turned
  up carrying the identical wrong constants, and all three had to move together:

  • _make_mptcp_capable (pcapkit/protocols/transport/tcp.py:2688) wrote
    length=20 if rkey is None else 32 where RFC 8684 section 3.1 gives 12 and 20 -- both
    branches wrong, the no-key branch writing the other case's value.
  • MPTCPCapable.rkey's own condition (pcapkit/protocols/schema/transport/tcp.py:709-711,
    pkt['length'] != 32) independently dropped the receiver's key for exactly the length the
    (also wrong) maker used to mean "key present", so the schema could not express a 12-octet,
    key-absent MP_CAPABLE at all.
  • The third site, easy to miss from TCP._make_mptcp_capable writes length 20/32 where RFC 8684 gives 12/20, and now packs wrong bytes rather than crashing #567's own text: _read_mptcp_capable
    (pcapkit/protocols/transport/tcp.py:1494 and :1508) rejected anything but
    schema.length in (20, 32) and read rkey only when schema.length == 32. This one is
    on the parse side and is only reachable once MPTCP.subtype is declared only under TYPE_CHECKING, so every subtype built via TCP's convenience constructor raises AttributeError #566 lets construction get that far --
    TCP's convenience constructor builds a schema and immediately reads it back through
    _read_mptcp_capable, so a caller building MP_CAPABLE through the public API hits this
    guard even though nothing has been unpacked from bytes yet. A reader who fixes only the
    maker and the schema predicate, going by TCP._make_mptcp_capable writes length 20/32 where RFC 8684 gives 12/20, and now packs wrong bytes rather than crashing #567's text alone, would still trip this guard.

  All three now agree on 12 if rkey is None else 20 / pkt['length'] == 20 /
  schema.length in (12, 20) and == 20.

This changes MP_CAPABLE's packed output. A caller who previously got a 20-octet option now
gets 12 (no key) or 20 (with key) depending on which they actually meant; a caller relying on
the old 32-octet, key-present form will see 20 octets instead, with the key actually on the
wire this time.

A robustness gain worth naming, not just a byte-count fix: parsing a spec-correct 12-octet
MP_CAPABLE used to depend on the #431/#578 short-read accommodation to survive at all. Measured
with only MPTCPCapable.rkey's predicate varied and the rest of the fix held constant: the old
pkt['length'] != 32 treats rkey as present for a 12-octet option too (12 != 32), so parsing
one tries to read 8 octets of receiver's key that are not there -- an 8-octet over-read that
warned packet length < 0: -8 and only "succeeded" because #431/#578 zero-pads a short read
rather than raising. The shipped pkt['length'] == 20 parses the same spec-correct 12-octet
option with no warning at all, because it no longer asks for octets the option never carried.

What #566 leaves recorded

MPTCP's class body now has no TYPE_CHECKING-only data attribute left: kind/length
became real fields in #565, and subtype is populated by the construction path here. The
if TYPE_CHECKING: def __init__(...): ... blocks in the concrete subclasses are ordinary
typed-signature stubs for Schema's synthesised __init__, not instances of this pattern, so
this was the last one.

Coverage

• tests/protocols/transport/test_tcp_mptcp_subtype_unit.py (new): builds each constructible
  MPTCP subtype through the public TCP() convenience constructor -- not _make_mptcp_*
  directly -- and asserts .subtype round-trips. MP_JOIN is excluded (separate,
  pre-existing no attribute '_flags' defect); MP_FASTCLOSE gets its own test asserting it no
  longer fails on subtype specifically, since it still fails for an unrelated reason (below).
• tests/protocols/transport/test_tcp_mptcp_capable_length_unit.py (new): packed-byte
  assertions for both RFC 8684 forms (12 octets without the key, 20 with it) via the maker
  directly, via hand-built spec-correct octets spliced into a full segment and parsed back
  (bypassing makers entirely, so pack-side and parse-side bugs can't cancel out), and via the
  public TCP() constructor end to end.
• Every new test was confirmed to fail on the pre-fix code, with the specific
  AttributeError/byte-count mismatch each one names.
• tests/protocols/test_option_roundtrip_unit.py: six of the seven tcp-mptcp/*
EXPECTED_FAILURES entries fix(tcp): give MPTCP real kind/length fields, repairing pack and parse #565 re-pointed at the subtype defect (MP_CAPABLE,
  ADD_ADDR, REMOVE_ADDR, MP_PRIO, DSS, MP_FAIL) now read 'OK' and are deleted.
  MP_FASTCLOSE does not: fixing subtype gets its construction past the AttributeError
  and into a second, independent defect -- its maker, its schema, and its own parser's length
  guard all disagree with each other (11 octets actually packed, 12 declared, 16 required to
  parse) -- filed as _make_mptcp_*/_read_mptcp_* length arithmetic is wrong at six more sites beyond MP_CAPABLE #576 and not touched here. MP_JOIN (no attribute '_flags') is
  unaffected and left alone.

Also found, not fixed here

While checking the sibling _make_mptcp_* helpers' length arithmetic, as #567 itself invited,
I found six more sites with the same shape of defect -- MP_FASTCLOSE (above), MP_JOIN SYN/ACK
and MP_JOIN ACK (wrong lengths, and SYN/ACK also drops hmac on reconstruction), REMOVE_ADDR
and MP_PRIO (length hardcoded rather than computed from what was actually given), and DSS
(declared length doesn't match what the schema's own ack/dsn fields actually pack). None of
these are #566 or #567, so none are touched here; all six are filed as #576 with exact
file:line citations and measured byte counts for each.

A pre-existing test encoded the bug as expected behaviour

tests/protocols/transport/test_tcp_udp_unit.py::TCPUDPUnitTests::test_tcp_mptcp_readers_cover_subtype_and_error_branches
(predates this PR and #565, last touched in #431/#432) hand-marks MP_CAPABLE schemas with
explicit lengths to exercise _read_mode_mp directly. It used 20 for the "no receiver" case, 32
for "with receiver", and 12 for an "invalid length" case that expects ProtocolError -- exactly
the pre-#567 (wrong) split, so it was pinning the defect rather than correct behaviour. Updated
to 12/20, with the invalid-length case moved to 32 (not an MP_CAPABLE length either way, so it
stays a genuine rejection).

Changelog

docs/source/changelog/1.5.0.rst updated, CHANGELOG.md regenerated with
python util/changelog_md.py (never hand-edited) and verified with --check.

Build/test

tests/protocols (528 passed, 1412 subtests) and the full tier (pytest tests) both pass in
full, run four times as main kept moving underneath this branch while it was in flight: once
before this PR's two new test files were made immune to a pre-existing test-suite hazard (see
below), once after (1281 passed, 17 skipped, 2850 subtests), once after rebasing onto main
post-#565-merge (1318 passed, 17 skipped, 2855 subtests), and once more after a second rebase
onto main post-#571/#572-merge (1320 passed, 17 skipped, 2857 subtests, exit 0 -- current
head).

One thing surfaced along the way that is worth recording even though it isn't a defect in this
PR's own code: the two new test files initially failed only when run as part of the full
suite (never in isolation), with FieldValueError: Field options has invalid value from
isinstance(item, Schema) returning False against an instance of a same-named class. Root
cause: tests/_support.purge_modules pops pcapkit's submodules out of sys.modules
elsewhere in the suite, and a later fresh import re-creates them as new objects; a name this
PR's test files bound to pcapkit at collection time (module-level imports) could end up
pointing at a schema built from a stale Schema base class, while OptionField.pack's own
delayed import resolves fresh at call time. tests/protocols/transport/test_tcp_udp_unit.py
already works around this by importing everything pcapkit-related inside each test method;
both new files here now do the same.

This is the same class-identity-after-reload hazard as #525 (shipped as 2ee2912d3): a
module popped out of sys.modules and reimported mints a second, distinct class object with
the same qualified name, and isinstance against the wrong one silently answers False rather
than raising anything that points at the cause. #525's fix was to stop resolving SchemaField
through a function-local import re-run on every call, moving it to module level instead; this
PR's fix runs the opposite direction for the opposite reason -- these two test files needed to
resolve pcapkit fresh at call time, specifically because other tests in the suite reload it
mid-run and a module-level binding goes stale. Worth flagging as the same family rather than a
one-off: it is why test_tcp_udp_unit.py's local-import convention exists, and it is likely to
recur in any new test that constructs a full protocol object via the public API and gets placed
after a purge_modules user in collection order.

[JarryShaw]

❌ NEEDS CHANGES

Two prose-only fixes, both inside the new test files. No behavioural defect found — every load-bearing claim here was independently verified and holds, including the ones I set out to falsify. The code is right; three citations point at the wrong RFC figure.

Cross-review on a different model from the author's, per the standing rule. Reviewed head 8acdb8387 — one commit on main@f6721fe66, genuine rebase, nothing behind. I also confirmed its tree is byte-identical to the earlier cfde459d9, so the long runs below (which started against that head) apply unchanged.

---

Required changes

1. RFC 8684 figure 5 is MP_JOIN, not MP_CAPABLE — 3 occurrences

tests/protocols/transport/test_tcp_mptcp_capable_length_unit.py lines 67, 148, 215 attribute the 20-octet key-present MP_CAPABLE form to "RFC 8684 figure 5".

Checked against the RFC text itself:

• Figure 4 — Multipath Capable (MP_CAPABLE) Option (§3.1)
• Figure 5 — Join Connection (MP_JOIN) Option (for Initial SYN) (§3.2)

RFC 8684 has exactly one MP_CAPABLE figure. Both forms this PR is about come out of Figure 4, which carries them as conditional rows — Option Sender's Key (if option Length > 4) and Option Receiver's Key (if option Length > 12) — plus the enumerated per-packet list in the same section. So "figure 4" at lines 59/134/192 is right but describes both forms, while "figure 5" at 67/148/215 sends a reader to a different option entirely.

Worth fixing rather than waving through, because this PR's whole thesis is that the previous code misread RFC 8684 §3.1's numbers — a wrong figure reference in the test that pins the correction is where it costs most. The production comments are all correct (they cite :rfc:8684`` section 3.1 with no figure number), as are both changelog entries, so this is contained to the one test file.

2. assertIn('invalid format', ...) is the fragment this PR itself calls unspecific

tests/protocols/transport/test_tcp_mptcp_subtype_unit.py:225

self.assertIn('invalid format', str(ctx.exception))

Gap.fragment's docstring in tests/protocols/test_option_roundtrip_unit.py — which this PR edits — says of exactly this string: "'invalid format' alone occurs 205 times across 13 modules (31 in internet/hip.py, 26 in transport/tcp.py), so it is satisfied by a regression at any of them." The PR applies that rule correctly in the EXPECTED_FAILURES entry ('TCP: [OptNo 30] invalid format') and then doesn't here. Same string is available; one line.

---

What I verified, and how

RFC 8684 §3.1 derived independently — 12 and 20 both confirmed

Derived from the RFC rather than from the PR. Figure 4's fixed head is Kind (8) + Length (8) + Subtype (4) + Version (4) + flags A..H (8) = 4 octets; each key is 64 bits = 8 octets. The section's own enumeration:

packet	contents	Length
SYN (A→B)	first 4 octets only	4
SYN/ACK (B→A)	B's key	12
ACK, no data (A→B)	A's key then B's key	20
ACK with first data	+ Data-Level Length, optional Checksum	22 or 24

So 4 + 8 = 12 without the receiver's key and 4 + 8 + 8 = 20 with it. length=12 if rkey is None else 20 is correct, and 32 is not an MP_CAPABLE length under any form — the valid set is {4, 12, 20, 22, 24}, which makes the old 20/32 pair wrong in both branches exactly as claimed.

Measured on the reviewed tree, packed through the maker and through TCP():

no rkey   -> 12 octets  1e0c010000000000000000aa   length octet = 12
with rkey -> 20 octets  1e14...                    length octet = 20

and hand-built spec octets parsed back with no maker involved: length=12 rkey=None, length=20 rkey=0xbbbbbbbbbbbbbbbb.

The fix is complete across all three sites — grep for not in (20, 32), length == 32, length != 32 over pcapkit/ returns nothing; the only surviving 32s are inside the comments describing the old values.

The construction-path choice over a real field is sound — double-packing confirmed empirically

This is the claim I most expected to break, and it holds. All 11 concrete MPTCP subclasses declare test as BitField(length=1, namespace={'subtype': (0, 4), ...}), so the subtype bits are already packed by every subclass. I built a variant declaring subtype as a real UInt8Field() on MPTCP and packed both:

shipped :  1e0c0181aaaaaaaaaaaaaaaa      12 octets   octet[2]=0x01 (subtype<<4 | version)
variant :  1e0c000181aaaaaaaaaaaaaaaa    13 octets   octet[2]=0x00  octet[3]=0x01

A real field adds an octet, encodes the subtype twice, shifts every following field by one, and leaves the declared length=12 disagreeing with 13 packed octets. So the stated reason is not a rationalisation, and _make_mode_mp — the single choke point all 11 makers return through — is the right place for it.

One correction to the comment's wording rather than its conclusion: corekit.fields does have a zero-pack field kind, NoValueField (length == 0, pack() → b''), already used by hopopt.py/ipv6_opts.py. It gives "don't pack" but not "derive" — its unpack returns None, so it would still need post_process on the parse path plus a subtype= in all 11 makers. The sentence is defensible on the word derive; it just overstates slightly. Not worth a revision on its own.

No fourth layer of the TYPE_CHECKING pattern

Of the 11 if TYPE_CHECKING: blocks in the MPTCP family, exactly one is a data attribute (subtype); the other ten are def __init__(...) signature stubs — the PR's characterisation is exact. I also checked _MPTCP.post_process's other assignment, ret.option: schema/transport/tcp.py:608 is the only occurrence of .option as an attribute anywhere in pcapkit/, so nothing reads it. A dead write, not a latent AttributeError waiting on the construction path.

The six deleted EXPECTED_FAILURES entries genuinely pass

Run, not read — driving examples.generators.options.outcomes() directly:

tcp-mptcp/ADD_ADDR        OK
tcp-mptcp/DSS             OK
tcp-mptcp/MP_CAPABLE      OK
tcp-mptcp/MP_FAIL         OK
tcp-mptcp/MP_PRIO         OK
tcp-mptcp/REMOVE_ADDR     OK
tcp-mptcp/MP_FASTCLOSE    CONSTRUCT   ProtocolError: TCP: [OptNo 30] invalid format
tcp-mptcp/MP_JOIN         CONSTRUCT   AttributeError: 'TCP' object has no attribute '_flags'

All six measure OK. The re-pointed MP_FASTCLOSE matches its recorded status and fragment exactly, and MP_JOIN is correctly left alone. Its three file:line citations all verify: transport/tcp.py:1893 is if schema.length != 16:, transport/tcp.py:3054 is length=12,, schema/transport/tcp.py:907 is MPTCPFastclose.test. MPTCPFastclose packs kind(1)+length(1)+test(1)+key(8) = 11 octets against a declared 12 and a parser demanding 16, so #576's measurement is right too.

On fragment specificity: [OptNo 30] is the Multipath TCP option kind and so is shared by every _read_mptcp_* reader, making it less discriminating than the docstring's own [OptNo 28] example. But the message carries nothing further, so this is "as specific as the message allows" — the rule as written — and the three file:line citations in defect carry the precision instead. No change wanted.

The modified pre-existing test was corrected, not weakened

test_tcp_udp_unit.py::test_tcp_mptcp_readers_cover_subtype_and_error_branches: every assertion is unchanged — assertTrue(capable.flags.req), assertTrue(capable.flags.hsa), assertIsNone(capable.rkey), assertEqual(..., 2), assertRaises(ProtocolError). Only the three marked lengths moved, 20/32/12 → 12/20/32, and 32 is genuinely not an MP_CAPABLE length under either split, so the rejection case stays a real rejection. capable_no_receiver also keeps rkey=2 while marked length 12, which makes it a stronger test than passing rkey=None would: it proves the reader keys off length rather than off whether rkey happens to be populated.

The purge_modules fix holds in isolation and in a full run

This was the point, since the two disagreed before. Both new files have zero module-level pcapkit imports (17 and 15 local imports instead), matching test_tcp_udp_unit.py's convention.

• isolation: 14 passed — exit code 0
• full tier on the reviewed tree: --collect-only finds the same 14 tests inside the whole suite (1337 collected), and the full run is 1320 passed, 17 skipped, 2857 subtests passed in 1196.79s — exit code 0, read from file

That reproduces the PR's own reported figures exactly.

Every new test fails without the fix

Reverted only pcapkit/protocols/schema/transport/tcp.py and pcapkit/protocols/transport/tcp.py to main and re-ran the two new files: 14 failed, 14 of 14 — exit code 1. Restored; git diff empty. The coverage claim is real in both directions.

Cited line numbers — all six verified by reading content at each position

Read out of 8acdb8387's own objects, not grepped:

citation	content
schema/transport/tcp.py:667	subtype: 'Enum_MPTCPOption'
schema/transport/tcp.py:709-712	the rkey ConditionalField, with lambda pkt: pkt['length'] == 20 at 711
transport/tcp.py:1494	if schema.length not in (12, 20):
transport/tcp.py:1508	rkey=schema.rkey if schema.length == 20 else None,
transport/tcp.py:2617	schema.subtype = subtype_val
transport/tcp.py:2688	length=12 if rkey is None else 20,

The changelog conflict resolution dropped nothing

docs/source/changelog/1.5.0.rst goes from 39 entries on main to 40 here. Set-differencing the entry lines: nothing present on main is missing from the PR, and exactly one entry is added. No other PR's entry was lost. util/changelog_md.py --check passes, and CHANGELOG.md agrees with the .rst.

The committed fixtures are intact

All six tracked blobs under examples/captures/ are byte-identical between f6721fe66 and 8acdb8387 — dhcp.pcapng 530c64cee… (1508 bytes) and in.pcap c4aac5acd… (605 bytes) included — and the diff does not touch that directory at all. The generated options-*.pcap captures are gitignored (.gitignore:8), so MP_CAPABLE's changed bytes leave no stale committed fixture either.

Bonus: this PR removes a dependency on the #431/#578 short-read accommodation

Not claimed in the PR, and worth recording since #578 just landed on that path. Parsing a spec-correct 12-octet MP_CAPABLE with only the rkey predicate varied:

shipped (length == 20) -> length=12 rkey=None   no warnings            clean
old     (length != 32) -> length=12 rkey=None   packet length < 0: -8  8-octet over-read

Under the old predicate a 12-octet option marked rkey present and read 8 octets that were not there, surviving only because FieldBase.unpack left-pads a short read with zeros — the exact accommodation #578 pinned. The new predicate reads precisely 12. #578's own new tests use an unassigned option kind (0x4f → UnassignedOption) and so never reach MPTCP; nothing here changes that path.

The PR body's claims check out

It names the third parser site, names the pre-existing test it corrected, and the #525 parallel is accurate: 2ee2912d3 is real (fix(corekit): stop a malformed TCP SACK's exception type depending on sys.modules state (#525) (#562)), and the body correctly notes that #525 moved an import to module level while these two test files move away from it — opposite directions serving the same class-identity-after-reload hazard. The one remaining mention of stacking is accurate historical narration ("originally branched from #565's head… has since been rebased onto current main"), not a live instruction.

---

Observations — not blockers, no action needed

• rkey defaults to 0, not None (_make_mptcp_capable, pre-existing and untouched here), so omitting rkey yields the 20-octet with-key form carrying an all-zero key; the 12-octet form needs an explicit rkey=None. Measured both. The PR anticipates this with test_default_rkey_of_zero_is_still_the_with_key_form, and the roundtrip generator's MP_CAPABLE override is {'skey': …} with no rkey, exactly as that test's docstring says — so tcp-mptcp/MP_CAPABLE exercises only the 20-octet form, and the 12-octet form's coverage comes from the new file. Optional[int] = 0 is a semantic oddity for a field whose presence changes the wire length, but it is out of scope here.
• RFC-legal lengths 4, 22 and 24 are all rejected (ProtocolError, measured). Pre-existing — the old (20, 32) guard rejected them too — and this PR strictly improves matters by admitting 12 and dropping the bogus 32. Note the RFC's own predicate for the receiver's key is Length > 12, which would extend naturally to 22/24; == 20 is narrower but exactly equivalent inside the admitted set. Possible follow-up, not a change request.
• tcp-mptcp/DSS now measures OK while warning packet length < 0: -8 — a real over-read, tracked by _make_mptcp_*/_read_mptcp_* length arithmetic is wrong at six more sites beyond MP_CAPABLE #576. I nearly raised this, since the module's docstring says a defect that leaves the cycle closed should be "pinned as tests of their own". It is not a standard this PR uniquely breaches: 6 of 274 OK cases already carry that warning (httpv2-frame/SETTINGS and four mh-extension/*, all pre-existing and untouched), so DSS joining them is consistent with existing practice.

Measurement discipline

All runs used PYTHONSAFEPATH=1 with PYTHONPATH at the review worktree root and the repo venv (3.14.7), asserting pcapkit.__file__ started with that root before importing anything else — printed and checked on the reviewed tree specifically. Exit codes were read from files, never from a summary line. No coverage run.

Both required fixes are prose-only and need no behavioural re-verification beyond re-running the two files.

[JarryShaw]

✅ GOOD TO MERGE

Re-point of my earlier review, which was NEEDS CHANGES on two prose items. Both are fixed correctly. Head a6d746c6b, one commit on main@f6721fe66.

git diff 8acdb8387 a6d746c6b -- pcapkit/ is empty, confirmed here — no source moved, so the whole behavioural verification from my previous comment carries forward unchanged: the RFC 8684 §3.1 derivation of 12/20, the empirically-confirmed double-packing (12 → 13 octets with a real subtype field), the six deleted EXPECTED_FAILURES entries all measuring OK, the corrected-not-weakened test_tcp_udp_unit.py, and the full tier at 1320 passed, 17 skipped, 2857 subtests. The delta is 2 test files, +11/−5.

Fix 1 — figure citation: correct, and the note is accurate

figure 5 is gone entirely — zero occurrences in the file. All six citations now read figure 4, at lines 59, 67, 135, 149, 193, 216.

The anti-copy-paste note at 67–69 states the RFC's structure correctly:

RFC 8684 figure 4's other conditional row (there is one MP_CAPABLE figure, not two -- both length forms come from it)

That matches Figure 4 as written: it carries the sender's key as (if option Length > 4) and the receiver's key as (if option Length > 12), so the 20-octet form genuinely is the other conditional row relative to the 12-octet form's, and both length forms do come from the one figure. This is the right shape of note — it explains why there is no figure 5 to cite, which is what stops the error coming back.

One loose edge, not worth another round: Figure 4 has a third conditional element too (the Data-Level Length / optional Checksum row, lengths 22/24), so "the other conditional row" is strictly "the other of the two key rows". In a file whose two constants are precisely the 12- and 20-octet forms that is unambiguous.

Fix 2 — fragment: matches the emitted message exactly

Confirmed by running it rather than reading it. build_mptcp_option(MP_FASTCLOSE, key=9) raises ProtocolError whose str() is:

'TCP: [OptNo 30] invalid format'

The new fragment is that string in full — it pins the entire message, not a substring, which is as specific as this assertion can be. Two things I checked because they would have made a specific-but-wrong fragment:

• schema.kind renders as the integer 30, not an enum repr like Option.Multipath_TCP, so [OptNo 30] is stable rather than dependent on __str__.
• assertNotIn('subtype', ...) on the next line still holds against the same message.

Both amended files pass: 14 passed, exit code 0.

The NoValueField wording

Agreed as a follow-up, not a blocker. Keeping this round to test files is what preserves the empty pcapkit/ diff, and that is worth more than the word change — it is why none of the behavioural verification needed redoing. For whoever picks it up: the comment on MPTCP says corekit.fields has no "derive, don't pack" field kind; NoValueField (length == 0, pack() → b'', used in hopopt.py/ipv6_opts.py) is "don't pack" without the "derive", so the sentence is defensible on that word but overstates. The decision it justifies is correct either way, as the double-pack measurement shows.

The robustness claim now in the body is accurate

Checked, since it is now a public claim. Every element holds against my own independent measurement: the old pkt['length'] != 32 does treat rkey as present for a 12-octet option (12 != 32); that does request 8 octets of receiver's key that are not there; it does warn exactly packet length < 0: -8; it survives only because #431/#578 zero-pads a short read; and the shipped == 20 parses the same octets warning-free. The body is also careful to scope it — "with only MPTCPCapable.rkey's predicate varied and the rest of the fix held constant" — which is the right qualification, since under the old complete code the (20, 32) guard rejected a 12-octet option before the schema-level over-read mattered. Good that it was reproduced independently rather than taken from my comment.

---

Nothing outstanding. Unpublished-equivalent caveat does not apply here; merging is the maintainer's call.