fix(tcp): seed TCP.read's flag accumulator with Enum_Flags(0), not a no-op cast (#616)

[JarryShaw]

TCP.read seeded its connection-flag accumulator with cast('Enum_Flags', 0). typing.cast is a runtime no-op — it returns its second argument unchanged — so the accumulator began life as the plain int 0, and the |= in the loop below it is the only thing that ever promotes it to a Flags member. A segment whose flags octet is all zero promotes nothing, so self._flags stayed an int and a membership test against it raised TypeError instead of answering.

Fixes #616.

Re-verified on current main

The issue's line numbers had shifted. Measured on ead73b204 (the merge base), CPython 3.14.7:

seed site  line 481        : _flag = cast('Enum_Flags', 0)      <- read path, the defect
seed site  line 588        : _flag = Enum_Flags(0)              <- make path, already fixed by #597

flagless (0x00):
    type(_flags)   = int
    repr(_flags)   = 0
    info.connection= 0
    Enum_Flags.SYN in _flags -> TypeError: argument of type 'int' is not a container or iterable

SYN only (0x02):
    type(_flags)   = Flags
    repr(_flags)   = <Flags.SYN: 16384>
    Enum_Flags.SYN in _flags -> True

So the issue's :481 is real and :563 is now :588 and was already fixed — only the read path remained.

Enum_Flags(0) is sound here — it is not the #623 defect

#623 is a _missing_ in pcapkit/const/mh/*_flag.py that ends return cls(value) with no extend_enum fallback, so F(0) recurses. pcapkit/const/tcp/flags.py declares no _missing_ at all, so Flags(0) is an ordinary aenum.IntFlag pseudo-member. Measured side by side:

Flags defines its own _missing_: False
Flags(0) OK -> <Flags: 0> | type = Flags | int = 0
  0 == Flags(0)              : True
  Flags.SYN in Flags(0)      : False
  Flags(0) | Flags.SYN       : <Flags.SYN: 16384>

MH BindingACKFlag(0) -> RecursionError  <-- this is #623

#623 does not block this PR, and no file under pcapkit/const/mh/** is touched.

Failing, then passing

Both sides run from immutable git archive snapshots in /tmp with the file hashed each side. The test file is byte-identical across the two runs (6a5afc06…); only tcp.py differs.

Before — tcp.py 0643bdd4…:

SUBFAILED(flags_octet=0) tests/protocols/transport/test_tcp_udp_unit.py::TCPUDPUnitTests::test_a_flagless_segment_seeds_its_connection_flags_as_an_enum
FAILED tests/protocols/transport/test_tcp_udp_unit.py::TCPUDPUnitTests::test_a_flagless_segment_seeds_its_connection_flags_as_an_enum
2 failed, 17 deselected, 1 warning, 3 subtests passed in 1.43s
### EXIT CODE FROM FILE: 1

with, at both the subtest and the dispatcher block:

>       self.assertIs(type(proto._flags), Enum_Flags)
E       AssertionError: <class 'int'> is not <aenum 'Flags'>

The three flagful subtests (0x02, 0x10, 0x12) pass on that side, so the test is failing for the reason claimed rather than incidentally.

After — tcp.py 85969673…:

1 passed, 17 deselected, 4 subtests passed in 0.67s
### EXIT CODE FROM FILE: 0

Exit codes are read from a file, not a pipeline.

Rebased twice during this work as main moved — onto ead73b204 (#628) and then cfb81d3f6 (#630) — both conflict-free. The whole transport tier after the final rebase: 143 passed, 103 subtests passed, exit 0.

Coverage cannot see this; the subtest count is the evidence

Line 481 already executed, so a fix to it moves no coverage number. Measured with coverage run over tests/protocols/transport/:

	merge base ead73b204	this PR
tcp.py stmts / branches	594 / 198	594 / 198
tcp.py coverage	100%	100%
tests passed	142	143
subtests passed	99	103

Coverage does not go backwards; +1 test and +4 subtests are the real delta.

What changes, and what does not

Nothing a caller can reach changes. _read_mptcp_join picks between RFC 8684 §3.2's three MP_JOIN layouts from these very membership tests, but mptcp_data_selector rejects a flagless MP_JOIN in the schema layer first — measured identical either side:

flagless (0x00)  -> FieldError: TCP: [OptNo 30] 1 invalid flags     (before AND after)
SYN (0x02)       -> parsed; options = [(Option.Multipath_TCP, 'MPTCPJoinSYN')]

That is what made the TypeError latent rather than live — latent only by virtue of a guard in a different file. Called directly on a flagless parsed segment the dispatcher now reaches the library's own error:

before: _read_mptcp_join -> TypeError: argument of type 'int' is not a container or iterable
after:  _read_mptcp_join -> ProtocolError: TCP: : [OptNo 30] 1: invalid flags combination

The one observable difference is the type of the reported connection for a flagless parsed segment:

before: to_dict()["connection"] = 0             (type int)
after:  to_dict()["connection"] = <Flags: 0>    (type Flags)

Numerically equal (0 == Flags(0)), identical on the wire (bytes(header) round-trips on both sides), and now conforming to what TCP.connection and Data_TCP.connection have always annotated. Flagful segments are unchanged.

That difference reaches the dump output, and it is worth stating plainly

The cross-review (below) caught that the first draft of this PR understated it. It is not merely "no longer an int" — the rendered value changes, in all three output formats:

json   flagless (0x00)   before: "connection": 0
json   flagless (0x00)   after:  "connection": "Flags::None [0]"
json   ACK (0x10)        before & after: "connection": "Flags::ACK [2048]"

tree   flagless (0x00)   before: |-- connection -> 0
tree   flagless (0x00)   after:  |-- connection -> Flags::None [0]

Two things about that:

• It removes a type inconsistency rather than introducing one. connection was a JSON number for a flagless segment and a JSON string for every other segment. It is consistently a string now.
• The literal None is a pre-existing defect elsewhere, newly exposed. pcapkit/dumpkit/common.py:216 builds f'{type(o).__name__}::{o.name} [{o.value}]', and Flags(0).name is None because a bitless aenum.IntFlag pseudo-member has no name. That would spell any zero-valued flag enumeration in the library the same way. dumpkit/common.py is outside this change's scope and fixing it would alter output for every such enum, so it is left to its own change and instead pinned by an assertion here so it cannot drift unnoticed.

The committed example dumps are unaffected: examples/captures/out.json and out.txt carry connection only for flagful segments (Flags::ACK [2048], Flags::ACK|FIN [34816]) — in.pcap contains no flagless segment. No test compares against those committed files (tests/foundation/test_extraction.py writes its own out.json into a temp dir), so nothing golden depends on this either way.

tests/protocols/test_option_roundtrip_unit.py still passes (6 passed, 358 subtests, exit 0). EXPECTED_FAILURES was imported rather than grepped, since ** unpacking hides it: its 4 TCP/flag entries are Quick_Start_Response, User_Timeout_Option, and the two pcapng epb_flags/pack_flags ones — none related, none newly passing.

Staying clear of #612 and #627

• test(tcp): reach the MP_JOIN dispatchers through TCP(), not a hand-written _flags (#603) #612 (e55fe0c59) did not touch pcapkit/protocols/transport/tcp.py at all — it is test-only plus a docstring in the schema module. It added the mptcp_option helper and replaced six hand-written proto._flags = {Flags.SYN} fakes with real TCP()/make() calls, and it explicitly named this defect under "Also found, deliberately not fixed". The long NOTE above make's Enum_Flags(0) came from fix(tcp): resolve the connection flags before building the options (#587) #597 (83987fbca), not test(tcp): reach the MP_JOIN dispatchers through TCP(), not a hand-written _flags (#603) #612.
• test(tcp): reach the MP_JOIN dispatchers through TCP(), not a hand-written _flags (#603) #612 left a NOTE in the test file asserting that a parsed segment could not be used for the flagless case "because read still accumulates into cast('Enum_Flags', 0)". This PR makes that false, so the comment is corrected in place — comment text only, no assertion or proto.make(...) call changed. The neighbouring NOTE about the make path is still accurate and was left alone.
• The cross-review found two more stale passages this PR had missed, in tests/protocols/transport/test_tcp_mptcp_join_flag_ordering_unit.py (from fix(tcp): resolve the connection flags before building the options (#587) #597, untouched here): one stating "the read path still seeds with cast('Enum_Flags', 0) — measured: a flagless TCP(...) reports _flags as the plain int 0. That read path is deliberately unchanged", and one stating its cast "was deliberately left as it was". Both are now false and both are corrected — docstring text only, no assertion touched.
• test(tcp): expect the short-read padding on the tail, repairing main (#604) (#621) #627 (6c3d1b0d9) touched only test_a_truncated_option_still_parses_its_declared_length; test(tcp): retarget the last assertion pinning the old padding side (#604) #628 (ead73b204) landed during this work and retargeted one more assertion in it. Rebased onto ead73b204 with no conflict, and test(tcp): retarget the last assertion pinning the old padding side (#604) #628's assertEqual(unassigned.data, trailing + b'\x00' * zeroes) is intact.
• Added a test rather than modifying one.

Found, deliberately not fixed

1. pcapkit/dumpkit/common.py:216 spells a nameless flag member None. f'{type(o).__name__}::{o.name} [{o.value}]' renders a bitless aenum.IntFlag pseudo-member as Flags::None [0]. It affects every zero-valued flag enumeration in the library, not just TCP's, and fixing it would change dump output well beyond this issue. Pinned by a test here; wants its own issue.
2. _read_mptcp_join's error string has a stray doubled separator — f'{self.alias}: : [OptNo …' renders as TCP: : [OptNo 30] 1: invalid flags combination. Cosmetic, sits in a message other tests may match on, unrelated to the flag seed.
3. pcapkit/protocols/application/httpv2.py's post_process seeds flags = 0 and never promotes to its Flags subclass when no bits are set — arguably the same latent-int-seed shape as this issue, on stdlib enum rather than aenum. Found by the cross-review; not investigated further here and not in scope.

Verification provenance

PYTHONPATH cannot defeat this repo's editable install: the venv installs a MetaPathFinder mapping pcapkit to the main checkout, and a meta-path finder runs before sys.path. Homebrew's own stdlib sitecustomize.py also shadows a PYTHONPATH one. Every measurement above therefore strips __editable__* from sys.meta_path in-process before the first import pcapkit, puts the tree at sys.path[0], and asserts and prints what it got:

stripped meta_path finders : ['__editable___pypcapkit_1_4_1_post2_finder']
pcapkit.__file__           : …/.claude/worktrees/agent-ad6f50c98d7dc71af/pcapkit/__init__.py

Sample captures were regenerated with examples/generators/make_samples.py (a fresh worktree has none; without them 7 runtime-tier tests fail on FileNotFoundError, unrelated to this change). CHANGELOG.md was regenerated with python util/changelog_md.py, never hand-edited; --check exits 0.

[JarryShaw]

Cross-review verdict: NEEDS CHANGES → addressed

Per the standing rule that an agent-raised change gets a cross-review on a different model, this PR was reviewed by a Sonnet subagent (the PR was authored by Opus), briefed to falsify rather than bless and run strictly read-only. No model substitution was needed.

Its verdict, verbatim on its first line:

NEEDS CHANGES

Both findings were real. I reproduced each independently before acting on it, rather than taking the report on trust.

Finding 1 — two stale docstrings the PR missed. CONFIRMED, fixed.

tests/protocols/transport/test_tcp_mptcp_join_flag_ordering_unit.py (from #597, untouched by the first draft) asserted the pre-fix behaviour as current fact in two places:

• "the read path still seeds with cast('Enum_Flags', 0) — measured: a flagless TCP(...) reports _flags as the plain int 0. That read path is deliberately unchanged"
• "why its cast('Enum_Flags', 0) was deliberately left as it was — the make path needed the Enum_Flags(0) seed; this one does not"

Both false after this change. Both corrected — docstring text only, no assertion touched.

Finding 2 — the dump format change was understated. CONFIRMED, disclosed and pinned.

The first draft said only "no longer an int in a dump". That undersold it. Reproduced against both immutable snapshots, hashing tcp.py each side:

                 before (0643bdd4…)          after (85969673…)
json  flagless   "connection": 0             "connection": "Flags::None [0]"
tree  flagless   |-- connection -> 0         |-- connection -> Flags::None [0]
json  ACK        "connection": "Flags::ACK [2048]"   (unchanged)

Affects all three of the JSON, tree and PLIST outputs. Two points I'd add to the reviewer's framing, both measured:

• It removes a type inconsistency rather than adding one. connection was a JSON number when flagless and a string otherwise; it is consistently a string now.
• The literal None is not this change's defect. pcapkit/dumpkit/common.py:216 interpolates o.name, and Flags(0).name is None because a bitless aenum.IntFlag pseudo-member is nameless. It would spell any zero-valued flag enumeration in the library the same way.

Actions taken: the PR body and the changelog bullet now state the rendering change explicitly; a new assertion pins 'Flags::None [0]' so it cannot drift silently. dumpkit/common.py is not changed — it sits outside this change's files and fixing it would alter output for every such enum, so it is recorded as wanting its own issue.

Findings I did not accept as blocking

The reviewer's third item suggested optionally fixing dumpkit/common.py here. Declined, for the scope reason above, and recorded under "Found, deliberately not fixed" instead.

Confirmed by the review (independently derived, not just echoed)

Claim	Verdict
Flags(0) sound; pcapkit/const/tcp/flags.py has no _missing_; does not hit the #623 recursion (BindingACKFlag(0) → RecursionError)	CONFIRMED
Test fails before / passes after, for the stated reason; assertIs(type(...), Enum_Flags) is the only valid discriminator since 0 == Enum_Flags(0)	CONFIRMED
Octet→enum mapping in the test's cases is genuinely correct, not accidentally passing — traced through BitField.post_process, ack at bit-index 3 (0x10) and syn at 6 (0x02)	CONFIRMED
No parse regression; flagless MP_JOIN still stops at mptcp_data_selector's FieldError; bytes(header) round-trips; hash(Flags(0)) == hash(0); Flags is an int subclass so isinstance(x, int) is unaffected	CONFIRMED
Coverage 100%/100% (594 stmts / 198 branches) either side; 142/99 → 143/103	CONFIRMED
Changelog accurate, CHANGELOG.md in step, supported RST subset only, #616 not GH-616	CONFIRMED
House rules: one commit, author, Fixes #616, in-library exceptions, .rst docs	CONFIRMED

Not verified

The reviewer did not re-run tests/protocols/test_option_roundtrip_unit.py itself and flagged that as unverified on its side. I ran it: 6 passed, 358 subtests passed, exit 0, and imported EXPECTED_FAILURES rather than grepping it — its 4 TCP/flag entries (Quick_Start_Response, User_Timeout_Option, pcapng epb_flags/pack_flags) are unrelated and none newly passes.

It also surfaced, out of scope, that pcapkit/protocols/application/httpv2.py's post_process seeds flags = 0 and never promotes to its Flags subclass when no bits are set — plausibly the same shape as this issue on stdlib enum. Not investigated here; noted in the PR body.

This PR is unmerged and awaiting review; I have not merged, tagged or released anything.

[JarryShaw]

Rewritten: changelog entry moved out, rebased onto current main

This branch was force-pushed. Head is now 24d45c776, one commit on top of
7b0df4a9c (origin/main at the time of the push). It reports mergeable: true
again; the UNKNOWN state cleared as a side effect of the push.

What this replaced. The branch was at d3e39a496, an "Update branch" merge of
main into the PR made at 11:41. That merge has been superseded rather than lost:
it changed no code at all — verified, git diff af09f0e14 d3e39a496 over the
three code files is empty, and it touched nothing outside this PR's five files — so
its only content was a resolution of the changelog conflict, which is precisely what
removing the changelog makes unnecessary.

What was dropped: this PR's bullet in docs/source/changelog/1.5.0.rst, and the
CHANGELOG.md regeneration that came with it. Nothing else. The code patch is
byte-identical to what was reviewed at af09f0e14 — verified by diffing the old
and the new patch restricted to the non-changelog paths, which came back empty. The
commit message is byte-identical too, Fixes #616 included.

Where the entry went: #657, the shared long-lived changelog PR for the 1.5.0
cycle, verbatim — re-derived from af09f0e14 itself and confirmed byte-identical,
35 lines. It merges last.

This PR now touches pcapkit/protocols/transport/tcp.py and its two test files —
3 files, down from 5.

Why this mattered more here than on the other four

While the branch still carried its changelog bullet, it and #657 did not conflict
— they insert at different anchors — so merging both would have landed the #616 entry
twice, in both docs/source/changelog/1.5.0.rst and CHANGELOG.md. Measured on
the merged tree: 2 occurrences, and util/changelog_md.py --check exits 0 on it,
so no gate would have caught it. That is the same failure mode as the drift described
below. It is gone now: in a tree with all six merged, each of the five bullets appears
exactly once and --check exits 0.

Expect the checks to re-run

The 22 green checks were attached to af09f0e14 and are superseded by the force-push.
The requeue is the cost of the strip, not breakage.

Changelog drift and the matrix jobs will nevertheless fail, here and on any branch
cut from current main. main itself is drifted: 375e9d411 (#638) hand-inserted
three lines into the generated CHANGELOG.md instead of running
util/changelog_md.py. python util/changelog_md.py --check exits 1 on main and on
this branch, and the two changelog files here are byte-identical to main's — so
the failure is inherited, not introduced. The repair is the first commit of #657 and
can be cherry-picked ahead of the rest of that branch.