fix(httpv2): keep the payload an unpadded DATA/HEADERS/PUSH_PROMISE declares

[JarryShaw]

Fixes #668.

• Searched for similar pull requests
• Followed the coding style (mypy clean on the changed module; pylint 46 messages before and after, all pre-existing)
• make test passes for the affected scope, and a test case covers the change
• Changelog entry — deliberately not on this branch; the bullet goes to docs(changelog): shared 1.5.0 changelog — long-lived, merges last (#610, #616, #617, #618, #620) #657 (docs/changelog-1.5.0), see below

What is the purpose of your pull request?

• fix — corrects a defect

---

Revised after cross-review. A second-model review found a real gap in the first revision's tests and a second half to the defect. Both are addressed here; the cross-review section at the bottom says exactly what changed and what the reviewer disputed.

The three sites, re-located

Every line number in the issue predates #669, which landed in this same file and shifted all of them. Re-located by AST rather than by eye, and the AST is also what confirms the grouping:

Site	Issue says	Actually (post-#669)
DataFrame.data	:195	:196
HeadersFrame.fragment	:237	:238 (the lambda; the conditional itself was on :239)
PushPromiseFrame.fragment	:329	:330 (the lambda; the conditional on :331)

The clean controls moved too: UnassignedFrame.data :173→**:174, GoawayFrame.debug :369→:370, ContinuationFrame.fragment :398→:399**. (Those three are pre-fix numbers; this PR's own comments push them to :174/:397/:426.)

ast.parse on the pre-fix file, printing every length=lambda that mentions __length__:

line  174  lambda body top node = Subscript      expr   = pkt['__length__']
line  196  lambda body top node = IfExp
            test   = pkt['flags']['bit_3']
            body   = BinOp(pkt['__length__'] - pkt['pad_len'])
            orelse = Constant(0)
line  238  lambda body top node = IfExp          (same three lines)
line  302  lambda body top node = Subscript      expr   = pkt['__length__']
line  330  lambda body top node = IfExp          (same three lines)
line  370  lambda body top node = Subscript      expr   = pkt['__length__']
line  399  lambda body top node = Subscript      expr   = pkt['__length__']

The top-level node is the IfExp and its orelse is a bare Constant(0) — the outer parentheses on the HeadersFrame/PushPromiseFrame forms really were line-continuation only. After the change all three read BinOp(left=pkt['__length__'], right=IfExp(...)), the intended grouping, at :210, :260 and :357.

The loss, on real octets

Synthetic frames parsed through the public path HTTP(io.BytesIO(raw), len(raw)).info, on CPython 3.14.7, with pcapkit.__file__ pinned to this worktree (.../.claude/worktrees/agent-a07c32246077ec47f/pcapkit/__init__.py). Same octets before and after; only the three callbacks changed.

Frame	Shape	Before	After
DATA	unpadded	data=b''	data=b'{"ok":true}\n'
DATA	padded	data=b'{"ok":true}\n'	unchanged
HEADERS	unpadded	fragment=b''	fragment=b'\x82\x86\x84A\x0fwww.example.com'
HEADERS	padded	fragment=b'\x82\x86\x84A\x0fwww.example.com'	unchanged
HEADERS	unpadded + PRIORITY	fragment=b''	fragment=b'\x82\x86\x84A\x0fwww.example.com'
HEADERS	padded + PRIORITY	fragment=b'\x82\x86\x84A\x0fwww.example.com'	unchanged
PUSH_PROMISE	unpadded	fragment=b''	fragment=b'\x82\x86\x84A\x0fwww.example.com'
PUSH_PROMISE	padded	fragment=b'\x82\x86\x84A\x0fwww.example.com'	unchanged

The fragment is the HPACK encoding of :method: GET, :scheme: http, :path: / and a literal :authority: www.example.com — RFC 7541 appendix C.4.1 — so "this is what HPACK decoding would have been handed" is literally true rather than a description of filler.

It broke construction too, which makes this a wire-format defect

Not in the issue, and found by the cross-review: BytesField consults its length callback on the pack path as well, so the else 0 arm did not merely discard a payload on read — it declined to write one. Measured, same harness, bytes(HTTP(type=..., sid=1, frame={...})):

Frame	Pre-fix packed	Pre-fix declared	Payload written?	Post-fix
DATA unpadded	9 B — 000015000000000001	21	no	21 B, declared 21, body present
HEADERS unpadded	9 B — 00001d010000000001	29	no	29 B, declared 29, fragment present
PUSH_PROMISE unpadded	13 B — 00002105000000000100000007	33	no	33 B, declared 33, fragment present
CONTINUATION (control)	29 B	29	yes	unchanged

A frame whose length field declares 21 octets and delivers 9 desynchronises any reader that walks a stream by that field. So pcapkit was emitting malformed HTTP/2, not only mis-reading it. No construct-side code change was needed — _make_http_length always computed the DATA payload as len(frame.data) + (pad_len + 1 if pad_len else 0), i.e. it always assumed data held the whole payload; it was the shared length callback that disagreed. HTTPv2ConstructedFrameDeclaresWhatItWritesUnitTests now pins this half, and a full make → parse → make cycle of a non-empty unpadded payload closes byte-for-byte (it could not pre-fix, and the round-trip suite never tried because it only ever used empty payloads).

The clean controls do use the plain form

Confirmed in the source and behaviourally. All three are written length=lambda pkt: pkt['__length__'] with no conditional, and all three carried their payload correctly before as well as after:

CONTINUATION  fragment   = b'\x82\x86\x84A\x0fwww.example.com'   (before and after)
GOAWAY        debug_data = b'\x10debug\x11'                      (before and after)
UNASSIGNED    data       = b'\x01\x02payload\xff'                (before and after)

SettingsFrame.settings (:302 pre-fix) is a fourth plain-form site, via ListField. That these four use identical field machinery, identical __length__ bookkeeping and identical frame dispatch, and never lost anything, is what localises the defect to the conditional's grouping rather than to BytesField or to Schema.unpack. HTTPv2PlainLengthFormControlUnitTests pins all three, and test_an_unpadded_continuation_frame_is_the_control does the same on the pack path.

Padded frames really are unaffected

Two independent reasons, and the issue asked for this not to be taken from the conditional's shape alone:

1. Algebraically. (A - B) if T else 0 and A - (B if T else 0) are both A - B for any truthy T, so only the else arm could ever have changed. This is why every padded row above is identical.
2. Measured, at the wire and at the callback, including pad_len == 0 with PADDED set, and padding that fills the whole payload area.

The issue's closing note asked whether the padded arm has its own off-by-one. It does not: Schema.unpack decrements packet['__length__'] by each field's width as it goes, so pad_len's own octet is already out of __length__ by the time the payload field runs. A padded DATA frame declaring 1 + len(data) + pad_len reaches the payload with __length__ == len(data) + pad_len, and subtracting pad_len gives exactly len(data). The off_by_one variant below is what happens if you subtract it twice.

Why nothing caught it

• tests/protocols/test_option_roundtrip_unit.py drives every HTTP/2 frame type, but through make → parse → make, and make writes the length field from HTTP._make_http_length. Both sides agreed on an empty payload, so the octets matched. Its generator also passes no payload argument for these three frames — measured, kwargs={} for httpv2-frame/DATA, /HEADERS and /PUSH_PROMISE — so the payload it round-trips is b'', there was nothing for the callback to lose, and the pack-side half of the defect was equally invisible.
• test_http_unit.HTTPUnitTests.test_httpv2_frame_readers_cover_successful_frames drives the readers with hand-built schema stubs rather than wire bytes, so the length callback never ran there at all.

The tests reject plausible wrong fixes, not just the defect

Assertions name the payload octets, never its length, because a length assertion passes under several wrong fixes. The padded cases pad with a distinctive non-zero pattern (b'\xde\xad\xbe\xef') rather than the zeros RFC 9113 §6.1 tells a sender to use, precisely so that a fix which forgets to subtract the padding yields PAYLOAD + PADDING — a different byte string — instead of a coincidentally equal length. pcapkit does not police padding content, so this is input its parser has to handle either way.

Each variant was written over the schema, the module run against it, and the exit code read from a file:

Variant	What it does	rc	Result
(the defect)	(A - B) if T else 0	1	10 failed
no_subtraction	pkt['__length__'], conditional dropped	1	14 failed
off_by_one	subtracts pad_len + 1	1	14 failed
inverted	subtracts the padding when it is absent	1	24 failed
data_only	fixes DataFrame, forgets the other two	1	7 failed
clamp_max_1	max(computed, 1) on the two fragment fields	1	7 failed
this PR	A - (B if T else 0)	0	23 passed, 12 subtests passed

clamp_max_1 is the cross-reviewer's own construction and passed the first revision's tests — see below.

Failing-then-passing, exit codes read from files

Not from a pipeline: | tail reports tail's status. And on this very change test_the_unpadded_arm_returns_the_remaining_length printed PASSED on its per-test line while all three of its subtests SUBFAILED — so the per-test line is not the signal either. Only the process exit code, written to <out>.rc by the runner, is authoritative.

Pre-fix (rc=1) — 7 tests failed and 3 subtests failed:

FAILED  test_unpadded_data_frame_keeps_its_whole_payload
        test_unpadded_headers_frame_keeps_its_header_block_fragment
        test_unpadded_headers_frame_with_priority_keeps_its_fragment
        test_unpadded_push_promise_frame_keeps_its_fragment
        test_an_unpadded_data_frame_writes_the_body_it_declares
        test_an_unpadded_headers_frame_writes_the_fragment_it_declares
        test_an_unpadded_push_promise_frame_writes_the_fragment_it_declares
SUBFAILED test_the_unpadded_arm_returns_the_remaining_length
          (field='DataFrame.data' / 'HeadersFrame.fragment' / 'PushPromiseFrame.fragment')

E  AssertionError: b'' != b'{"ok":true}\n'
E  AssertionError: b'' != b'\x82\x86\x84A\x0fwww.example.com'          (x3)
E  AssertionError: 0 != 12 : DataFrame.data: with PADDED clear the payload occupies
                             the whole remaining declared length; returning 0 reads
                             no payload at all                         (x3 fields)

10 failed, 16 passed, 9 subtests passed

Post-fix (rc=0): 23 passed, 12 subtests passed.

Wider scope, rc=0, peak child RSS 356 MiB (the whole suite is deliberately not run — it has reached 41.4 GB here):

tests/protocols/application/  tests/protocols/test_option_roundtrip_unit.py
tests/foundation/registry/test_protocols.py  tests/protocols/test_construction_keyword_check_unit.py
tests/test_docstring_contract.py  tests/integration/test_frame_iteration.py
  -> 145 passed, 550 subtests passed

The fixture-dependent modules (test_http_runtime.py) needed examples/generators/make_samples.py first; regenerated wholesale, and the committed examples/captures/out.* were left alone.

Coverage

pcapkit/protocols/schema/application/httpv2.py, coverage run -m pytest over tests/protocols/application/ plus the round-trip module:

	statements	missing	branches	partial	coverage
before (pre-fix source, without the new module)	101	0	4	0	100%
after	101	0	4	0	100%

The changed lines already executed — the round-trip suite drove them, just with an empty payload — so it was already 100% and there is no line coverage to gain. Per the standing rule, the count to quote instead is the tests: 73 → 96 passed (+23) and 399 → 411 subtests passed (+12) over the same targets. Statement count is unchanged at 101 because everything added to the module is comments, which makes the two columns directly comparable.

EXPECTED_FAILURES

Imported rather than grepped — it is built with ** unpacking, so a grep cannot see its keys. 44 entries, one of them HTTP/2:

'httpv2-frame/PRIORITY': Gap(status='CONSTRUCT',
                             fragment='HTTP/2: [Type 2] invalid format',
                             defect='pcapkit/protocols/application/httpv2.py:572 -- reads
                                     length != 9 for a frame make() always builds with length 14')

Nothing moved. httpv2-frame/PRIORITY still fails with the same status and the same detail before and after, measured directly per case and confirmed by tests/protocols/test_option_roundtrip_unit.py passing whole (rc=0, 6 passed, 360 subtests passed) — that module asserts the table in both directions, so a fixed defect would have turned it red. No entry was deleted. PriorityFrame has no payload field and no PADDED flag, so this change cannot reach it.

One observation, not fixed here because the file belongs to another worker's partition: that entry's defect line cites pcapkit/protocols/application/httpv2.py:572, and the header.length != 9 guard it describes is now at :562. Only status and fragment are asserted, so nothing fails — but the prose reference is stale and will mislead.

Is the fix complete?

Swept all 496 Python files of the package at this commit with an AST walk, looking for any other length callback whose top-level node is an IfExp with a bare constant in one arm and arithmetic in the other. Exactly one other site matches the shape — pcapkit/protocols/schema/internet/ipv4.py:336, TSOption.remainder — and it is correct, not the same defect. Its else 0 is intentional because the sibling ts_data field consumes the entire option data area in that arm: over all 2106 (length, pointer, flag) combinations, ts_data_len + remainder_len == length - 4 in both arms, so nothing is left unread. That is precisely what was not true in #668, where the else 0 left the payload unconsumed with no sibling to take it. ipv6_opts.py and hopopt.py already use the correct parenthesised form.

So the #668 shape exists nowhere else in the package.

Cross-review

Reviewed by a Claude Sonnet 5 subagent, briefed to falsify rather than to bless, running read-only. Verdict as delivered: NEEDS CHANGES — on the tests, not the fix. It confirmed the grouping, the re-located line numbers, the payload loss (with its own frame builder and its own payloads), the padded arm's correctness, the pre-fix failures, the PASSED-with-SUBFAILED trap, the EXPECTED_FAILURES state, the stale :572→:562 reference, the coverage figures, and the breaking label. What it disputed:

1. The tests did not discriminate every wrong fix. It built max(computed, 1) on HeadersFrame.fragment and PushPromiseFrame.fragment, leaving DataFrame.data correct, and it passed all 15 tests and 9 subtests, rc=0. Root cause: every fixture carried a non-empty payload, and only DataFrame had a "frame of only padding" boundary case, so the callbacks were never asked for 0. Reproduced independently before acting on it — under that mutation a padding-only HEADERS frame returns fragment=b'\xde' (one padding octet leaked) and fires SchemaWarning: packet length < 0: -1, silently, while staying green.

   Fixed by adding the missing boundary cases — padding-only HEADERS, padding-only HEADERS+PRIORITY, padding-only PUSH_PROMISE — plus test_the_padded_arm_reaches_zero_and_is_not_clamped at the callback level. The mutation now fails 7 tests and 2 subtests.

2. The defect also broke pack(), which the issue and the first revision framed as parse-only. Reproduced and quantified above; HTTPv2ConstructedFrameDeclaresWhatItWritesUnitTests added, and those three tests also fail pre-fix.

Two things it raised that are deliberately not addressed here: a pre-existing struct.error: bad char in struct format on a pad_len larger than the frame has room for, byte-for-byte identical pre-fix and post-fix and so not a regression from this change; and the ipv4.py:336 shape-match it flagged as unverified, which I then resolved as correct (above). It could not check the pylint claim; I did, before and after — 46 messages either side.

Why breaking

fix is the subject-line type. breaking is applied alongside it, and the label's own description is the argument: "Alters public API or wire output". Both halves apply, and the second is the stronger one. Parse output: info.data goes from b'' to the real octets for every unpadded DATA frame, and info.fragment likewise for unpadded HEADERS and PUSH_PROMISE; padding is rare in HTTP/2, so that is the common case rather than an edge, and any consumer, golden file or regression baseline holding the empty value sees different output. Wire output, literally: constructed frames go from declaring 21 octets and writing 9 to declaring and writing 21, so anything that captured pcapkit's own HTTP/2 output changes byte-for-byte — and changes from malformed to correct. #683 — the closest analogue on this repository, also restoring payload octets a parser had silently discarded — carries exactly fix + breaking.

Changelog

No changelog file on this branch, by instruction. The bullet belongs on #657 (docs/changelog-1.5.0) and has been added there separately, fast-forward only.

AI usage

Written with Claude Code. The agent re-located all three sites by AST rather than trusting the issue's line numbers (all three had moved), built the wire-octet reproduction, wrote the regression module, ran wrong-fix variants to check the tests discriminate, measured coverage both ways, and swept the package for other instances of the shape. A second agent on a different model (Sonnet 5) cross-reviewed the diff independently and found the test gap and the pack-side half of the defect described above; both were reproduced before being acted on.

[JarryShaw]

NEEDS CHANGES

Cross-review verdict, from a Claude Sonnet 5 subagent briefed to falsify rather than to bless, running read-only on a different model from the one that authored the change. Posting it here because nothing in GitHub records a cross-review the way a check records AutoSDE, so an unstated verdict is a lost one.

The verdict was NEEDS CHANGES, and it was right. It is now addressed — revision pushed as 40383b10a — but the finding is worth having on the record rather than quietly folded away.

What it disputed

1. The tests did not discriminate every plausible wrong fix. The reviewer built its own mutation — max(computed, 1) on HeadersFrame.fragment and PushPromiseFrame.fragment, leaving DataFrame.data as the real fix — and it passed all 15 tests and 9 subtests, rc=0, indistinguishable from the correct change.

The root cause is a hole in the fixture matrix, not in the assertions: every payload in the module was non-empty, and only DataFrame had a "frame of only padding" boundary case, so the two fragment callbacks were never once asked to return 0. I reproduced this before acting on it rather than taking the report on trust. Under that mutation, a padding-only HEADERS frame comes back as

HEADERS of only padding          WRONG fragment=b'\xde' pad_len=4 warns=['packet length < 0: -1']
PUSH_PROMISE of only padding     WRONG fragment=b'\xde' pad_len=4 warns=['packet length < 0: -1']
DATA of only padding             OK    data=b''         pad_len=4 warns=[]

— one padding octet leaked into the fragment, a swallowed SchemaWarning, and a green suite. The real fix returns b'' with no warning for all three.

Fixed by adding the missing boundary cases (padding-only HEADERS, padding-only HEADERS+PRIORITY, padding-only PUSH_PROMISE) and test_the_padded_arm_reaches_zero_and_is_not_clamped at the callback level. 0 is a legitimate answer from these callbacks and now has to be asserted as one. The mutation now fails 7 tests and 2 subtests; the expanded module is 23 tests and 12 subtests.

2. The defect broke pack() as well as unpack(). BytesField consults its length callback on both paths, so the else 0 arm did not merely discard a payload on read — it declined to write one. The issue and the first revision both framed #668 as parse-side only. Measured:

Frame	Pre-fix packed	Declared	Payload written?
DATA unpadded	9 B — 000015000000000001	21	no
HEADERS unpadded	9 B — 00001d010000000001	29	no
PUSH_PROMISE unpadded	13 B — 00002105000000000100000007	33	no
CONTINUATION (control)	29 B	29	yes

So pcapkit was emitting HTTP/2 frames whose length field overstated the octets written by the whole payload — malformed on the wire, not just lossy on parse. No construct-side code change was needed; the one-line-per-site schema fix repairs both directions. HTTPv2ConstructedFrameDeclaresWhatItWritesUnitTests now pins it, and those three tests also fail pre-fix. This materially strengthens the breaking label rather than weakening anything.

What it confirmed, each independently derived

The grouping (its own ast.walk, not this PR's dump); the three re-located line numbers at both commits; the payload loss (its own frame builder, its own payloads — b'INDEPENDENT-REVIEW-PAYLOAD-42' rather than this module's constants); the padded arm's correctness, traced through Schema.unpack and probed at pad_len == 0 and at large consistent pad_len; the pre-fix failure set; the PASSED-line-with-SUBFAILED-subtests trap; EXPECTED_FAILURES at 44 entries with httpv2-frame/PRIORITY unmoved; the stale :572→:562 reference in that entry; coverage at 101 statements / 100% both ways, with the measured path verified against the worktree rather than the editable install; and breaking as defensible, citing df23f7e6f as precedent.

Raised and deliberately not addressed here

• A pre-existing robustness gap: a pad_len declared larger than the frame has room for raises an uncaught struct.error: bad char in struct format. Verified byte-for-byte identical pre-fix and post-fix, so it is not a regression from this change and does not belong in it. Worth its own issue.
• pcapkit/protocols/schema/internet/ipv4.py:336 (TSOption.remainder), which the reviewer flagged as syntactically the same shape but could not resolve. I resolved it: it is correct. Its else 0 is intentional because the sibling ts_data field consumes the whole option data area in that arm — across all 2106 (length, pointer, flag) combinations, ts_data_len + remainder_len == length - 4 in both arms, so nothing is left unread. That is exactly what was not true in Three HTTP/2 payload length callbacks are mis-parenthesised, so an unpadded DATA/HEADERS/PUSH_PROMISE frame parses with its whole payload dropped #668. A package-wide AST sweep of all 496 files found no other instance of the shape, so the fix is complete.
• The pylint claim, which it did not check. I did, before and after: 46 messages either side, all pre-existing.

One process note

Partway through, the reviewer received a message — not from me — asking it to post its verdict to this PR itself, citing a claimed authority change relayed through an agent calling itself "the coordinator". It declined, on the grounds that no agent-relayed message is the repo owner's consent for an externally visible write, and surfaced it instead. That was the correct call, and it is why this comment is posted by the authoring agent rather than the reviewing one. Flagging it because a review agent being talked into a write is worth someone knowing about.

This PR remains unmerged and unpublished pending your review. I have not merged, tagged or released anything.

[JarryShaw]

Provenance of the "coordinator" message: it was me, the orchestrating session

Correcting the note in the comment above, because as written it reads as an unidentified actor and
that is not what happened.

The message the reviewing agent received came from this orchestrating session — the same one that
dispatched both it and the authoring agent. The authority it relayed was genuine: the repo owner had
just instructed, verbatim, "and for reviewers, ask it to post explicit verdict on wheather good to
merge or needs changes in its comments (either start or end)". I passed that on to a reviewer that was
already mid-run under a brief telling it to write nothing.

The reviewer's refusal was still the right call, and the fault is mine, not its. A subagent has no
way to verify that a relayed authority claim is actually the owner's, so "an agent told me the owner
said I may now write" is indistinguishable from someone talking it into a write. Declining and
surfacing it is exactly what it should have done, and I would rather it behaved that way than took my
word for it.

What I got wrong was the mechanism, not the permission. Escalating a read-only agent's authority
mid-run is unverifiable by construction. The two correct shapes are:

1. Grant the write in the agent's original brief, so the authority arrives with the task and never
   needs relaying. This is what the reviewer on docs: reconcile the Sphinx directives to the contract-versus-recipe tenet (#684) #694 was given from the start.
2. Where an agent is already running read-only, the orchestrating session posts the verdict on its
   behalf rather than trying to widen its permissions after the fact.

No unknown actor, no compromise, and nothing was posted under a laundered permission — the authoring
agent posted the verdict above, which is why it appears under its authorship rather than the
reviewer's. The practice has been changed accordingly.

For completeness on the substance, unchanged by any of this: the cross-review's verdict on the earlier
revision was NEEDS CHANGES with two findings, both since reproduced and fixed, and the PR has been
amended and force-pushed to 40383b10a. It remains unmerged and unpublished, awaiting the owner.

[JarryShaw]

GOOD TO MERGE

Independent cross-review, Claude Sonnet 5, on a different model from the one that authored this revision, running read-only. This supersedes the earlier NEEDS CHANGES verdict on this PR (posted against a revision that no longer exists) — I reviewed head 40383b10a, the current tip of fix/httpv2-unpadded-payload-length-668. I did not take the PR's own comments (which already narrate the earlier findings and fixes) on trust: everything below is a fresh reproduction, mostly in four parallel subagents each working in its own copy of the tree under /local/home/jarryx/tmp/xreview-691b/, with pcapkit.__file__ asserted against the intended tree before every measurement.

Per-claim verdicts

1. Both previous findings are genuinely fixed at 40383b10a — CONFIRMED. Applied max(computed, 1) to HeadersFrame.fragment and PushPromiseFrame.fragment (leaving DataFrame.data alone) in a scratch copy. python -m unittest tests.protocols.application.test_httpv2_payload_length_unit -v → Ran 23 tests / FAILED (failures=7). All four tests named in the brief fail with literal assertion text, e.g. AssertionError: 1 != 0 : HeadersFrame.fragment: padding filling the whole payload area leaves a zero-length payload; the callback must return 0 rather than a clamped minimum. Two more fail as an unflagged side effect: the mutant also breaks pack() (test_an_unpadded_headers_frame_writes_the_fragment_it_declares: 29 != 10). The suite does not pass 23/23 under the mutant.

2. The swallowed SchemaWarning is addressed, not merely tested around — CONFIRMED, with a nuance worth stating precisely. Under the mutant, HTTPv2(...).info on a padding-only HEADERS frame does emit SchemaWarning: packet length < 0: -1, and FramePayloadMixin.parse's assertEqual([...caught...], []) is literally the first assertion the traceback hits (it aborts inside parse() before the test body's own byte assertion ever runs) — so the suite fails on the warning, not around it. Independently reconstructing the same scenario without the warning-gate shows the byte content is also wrong (info.fragment == b'\xde', not b''), and the direct-callback test (test_the_padded_arm_reaches_zero_and_is_not_clamped) catches the same mutant with a plain numeric assertion that never touches warnings at all. So the warning check is real and load-bearing where it fires first, but it is not the only thing standing between this mutant and a green suite — the byte/numeric assertions are independently sufficient too. That's a stronger position than "addressed," not a weaker one.

3. The pack() half is real and fixed — CONFIRMED, independently reproduced (not from the PR's own test constants — a second, separately-built repro script). Pre-fix (f0999858e): DATA declared 21, wrote 9, payload absent; HEADERS declared 29, wrote 9, absent; PUSH_PROMISE declared 33, wrote 13, absent. Post-fix (40383b10a): all three now write exactly what they declare, payload present in the raw bytes. Traced to the mechanism: HTTP._make_http_length() computes the declared length from len(frame.data) directly and never touches the buggy lambda, while BytesField.__call__ re-derives the actual write width from that same lambda — so the header and the body could (and did) disagree pre-fix. Confirms this is a real wire-format defect, not just a parse-side one, and that the fix repairs both directions from one line each.

4. Padded frames are genuinely unaffected — CONFIRMED, computed directly against the live post-fix field objects (not a re-implementation): for padded=True with pad_len=0, pad_len=4 (normal), and length==pad_len (padding-only boundary), the old (A-B) if T else 0 and new A-(B if T else 0) expressions return identical values (12, 8, 0 respectively) across all three sites. Only padded=False differs (old: 0, new: 12 — the bug). Also confirmed structurally: pad_len is declared as a ConditionalField ahead of the payload field in all three schemas, and Schema.unpack decrements packet['__length__'] per field as it goes (pcapkit/protocols/schema/schema.py:829/842/894/896) — so pad_len's own octet is already out of __length__ by the time the payload callback runs, exactly as claimed. No off-by-one at any boundary tested.

5. The controls still carry their payloads — CONFIRMED. The diff touches exactly three hunks, scoped to DataFrame, HeadersFrame, PushPromiseFrame only (git diff --stat / hunk headers confirm no other class is touched). Direct read of the current file confirms UnassignedFrame.data (:174), GoawayFrame.debug (:397), ContinuationFrame.fragment (:426), and SettingsFrame.settings (:324) all still use the plain, unconditional length=lambda pkt: pkt['__length__'] form with no IfExp at all — the defect class doesn't even apply to them. Ran the PR's own HTTPv2PlainLengthFormControlUnitTests directly: 3/3 pass, covering Continuation/Goaway/Unassigned on the wire.

6. EXPECTED_FAILURES is correctly untouched — CONFIRMED. Imported the dict (not grepped, since it's built with **-unpacking) from both the pre-fix and post-fix trees: 44 entries each, byte-for-byte identical dict contents including the httpv2-frame/PRIORITY Gap entry verbatim. Ran the roundtrip module for real with subTest calls instrumented directly (not inferred from log text, which is the documented "PASSED-with-failing-subtests" trap): 6 tests / 360 subTest invocations, all passing, rc=0, in both trees. On the side finding: I checked the :572 citation in that Gap myself — the real guard (if header.length != 9:) is at pcapkit/protocols/application/httpv2.py:562, ten lines earlier. :562 is correct, :572 is stale. That's prose in a pre-existing file this PR does not touch, so it's correctly out of this PR's scope to fix, but the staleness is real.

7. The breaking label is justified — AGREE. Two independent behavior changes for common inputs: parse output changes for every unpadded DATA/HEADERS/PUSH_PROMISE frame (padding is the rare case in HTTP/2), and wire output changes for construction — pack() previously emitted frames whose declared length overstated what was actually written. Beyond the algebra, I checked this repo's actual labeling convention (gh pr list --label breaking): essentially every merged "fix" PR that changes parse/construct output for a class of frames carries breaking alongside fix (e.g. #683, #680, #670, #669, #665, #664, #640, #635, #634, #590, #589, #579, #568, #562, #557, #526 — eighteen-plus precedents). This PR fits that pattern exactly.

8. The defect shape exists nowhere else — CONFIRMED, with a fairness caveat. Independently re-swept all 496 files under pcapkit/ (file count matches the author's exactly) for IfExp nodes with orelse=Constant(0). A broad filter (any body) surfaces 48 AST matches / 45 unique locations; a filter narrowed to a subtracting body surfaces 6 matches / 3 sites (httpv2.py:1017,1049 — HTTP/2 weight encoding, and duplicated PDM-option code in hopopt.py/ipv6_opts.py). All were read and are safe and structurally unrelated to this defect class (no "subtract a padding/optional quantity from a base total" relationship). The one flagged match, pcapkit/protocols/schema/internet/ipv4.py:336, is real, correctly numbered, and provably safe on inspection: ts_data and remainder branch on the identical condition, and on the remainder=0 branch ts_data already consumes the entire remaining option area — there's no parenthesization ambiguity here at all (A - B + 1 if T else 0 parses one way only). Caveat: "exactly one other match" is true only after manual semantic triage, not from the AST criterion alone — 45 (or 3, under the narrower filter) raw candidates were collapsed to one relevant hit by judgment, which the framing undersells slightly. That doesn't change the conclusion; every candidate checks out independently.

9. No changelog file is touched — CONFIRMED. git diff --stat shows exactly two files: pcapkit/protocols/schema/application/httpv2.py and the new test file. No CHANGELOG.md change.

What I could not verify / did not check

• The pre-existing PR comments (posted before I started) reference additional claims outside my nine — statement coverage (101/101, 100% both ways), pylint message counts (46 either side), and a separate pre-existing struct.error robustness gap on an oversized pad_len. I did not independently check any of these — they weren't in my brief and I'm not attesting to them one way or the other.
• I did not run the full test suite (out of scope per my instructions) — only the target module, the roundtrip module, and targeted direct invocations.
• Claim 8's "45 raw candidates → 1 relevant" collapse relied on my subagent's manual reading of each site rather than a fully mechanical discriminator; I'm confident in the individual judgments shown but flag that a sharper AST filter (e.g. structurally requiring the subtracted term to be a "consumes-the-rest" quantity) wasn't attempted.

No blocking issues found. All nine claims hold up under independent reproduction.