morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 07:47:29 +00:00

[PR #5762] [MERGED] fix(pcap): preserve frames with empty protocol stacks #7043

New issue

Closed

opened 2026-11-01 21:44:55 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-11-01 21:44:55 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/5762
Author: @gensokyo-dev
Created: 10/21/2026
Status: ✅ Merged
Merged: 10/21/2026
Merged by: @gensokyo-dev

Base: main ← Head: fix/5734-empty-pcap-protocols

📝 Commits (2)

2f3bbfc fix(pcap): preserve frames with empty protocol stacks
f494954 test(pcap): retain facts around empty protocol frames

📊 Changes

3 files changed (+100 additions, -9 deletions)

View changed files

📝 docs/pcap-analysis.md (+5 -1)
📝 src/pcap/PcapOutput.test.ts (+87 -0)
📝 src/pcap/PcapOutput.ts (+8 -8)

📄 Description

Summary

Fixes #5734. A valid TShark frame.protocols: [""] currently discards the entire capture summary. Keep those frames in the packet count, retain other frames' facts, and omit empty protocol names from the distribution.

Change and scope

Validate the single protocol-stack value once in PcapOutput and reuse it during aggregation, removing the redundant per-frame Set. CLI, MCP summaries, retained pages and full Evidence share this producer. Missing/multivalued fields, malformed output and invalid frame identities remain refused. Document the distinction between examined frame coverage and observed protocol names.

Existing static saved-capture inspection only; additional maintainer scope decision is not applicable. No new schema, authority, cache or resource-limit changes.

Evidence and validation

npm run test:focused -- src/pcap/PcapOutput.test.ts src/pcap/PcapConfiguration.test.ts tests/boundary/pcap/pcapProvider.test.ts tests/boundary/pcap/pcapRetainedView.test.ts tests/boundary/mcp/pcapMcp.test.ts tests/boundary/cli/pcapCli.test.ts — initial 85 tests passed. After review, npm run test:local -- src/pcap/PcapOutput.test.ts passes all 27 cases, including two additional endpoint/DNS preservation cases. The updated producer suite fails 6 cases on original main (335c042d1).
npm run check — passed typecheck, lint, format, knip, metadata and architecture guards.
npm run docs:check — passed, including 5 documentation tests.
Real Linux TShark/capinfos 4.0.17, public CLI and SDK stdio MCP: empty capture; all-zero-length, nonempty and mixed user0 captures; 20,000 mixed records; and the unchanged 43-frame Wireshark HTTP sample. All six pass after the fix. The three affected inputs fail with unreadable_output on original main.
Example restored result: two zero-length records → packet_count: 2, protocol_count: 0, protocol_distribution: [], coverage: "complete"; mixed two-record capture → one user_dlt:data observation. Native TShark fields and capinfos counts provide the independent oracle. Repeated summaries preserve parent identity; protocol pages and exported full Evidence match the CLI, target digests remain unchanged, and temporary roots are empty after close.
Review follow-up: native mergecap combines the HTTP sample and zero-length user0 records into a 45-frame pcapng. Actual CLI/MCP preserve all four endpoints, two HTTP URIs and the complete five-stack distribution; the native oracle, retained pages and full Evidence export agree. This additionally verifies the mixed real-traffic case, beyond the parser regressions with empty frames before/after a DNS response.

Performance and limits

Equivalent completed HTTP task (catalog, CLI, two MCP summary/page requests, full Evidence export and cleanup), one CPU: before/after 21.84/21.32 s elapsed, 21.33/20.95 s CPU, 1219.73/1271.83 MiB peak process-family memory. Fresh processes; OS caches uncontrolled. First MCP summary/page: 10.49/9.96 s; repeat: 1.59/1.28 s. This single pair establishes no speedup or memory reduction.

Catalogs are identical. Captured JSON-RPC cl100k_base estimates are equal at 894,123 tokens including the full catalog (884,676), arguments, replies and repeats; the incoming initialization reply and actual model usage are unmeasured. Input-only selected-tool schema estimate is 14,735 tokens. These are serialization estimates, not claimed agent token consumption or savings.

All qualified scopes use 8 GiB memory, a 7 GiB early-stop threshold, no swap and one CPU. Payload-exit cgroup receipts show zero OOM events and only the accounting observer remaining. Native Windows/macOS and other Wireshark versions remain unverified. No breaking change or package metadata change.

Author readiness

Human owner: @gensokyo-dev

The session's human owner explicitly delegated completion of these declarations and requested initial ready-for-review publication; this records that standing instruction, without claiming an additional manual human review occurred during this run.

I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work.
I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps.
I declare this PR ready for initial maintainer review.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/5762 **Author:** [@gensokyo-dev](https://github.com/gensokyo-dev) **Created:** 10/21/2026 **Status:** ✅ Merged **Merged:** 10/21/2026 **Merged by:** [@gensokyo-dev](https://github.com/gensokyo-dev) **Base:** `main` ← **Head:** `fix/5734-empty-pcap-protocols` --- ### 📝 Commits (2) - [`2f3bbfc`](https://github.com/morluto/rea/commit/2f3bbfc5c6db8456095836d1e07f878ae914813f) fix(pcap): preserve frames with empty protocol stacks - [`f494954`](https://github.com/morluto/rea/commit/f4949540c359c0d14275307ac745c6eb62c33f8e) test(pcap): retain facts around empty protocol frames ### 📊 Changes **3 files changed** (+100 additions, -9 deletions) <details> <summary>View changed files</summary> 📝 `docs/pcap-analysis.md` (+5 -1) 📝 `src/pcap/PcapOutput.test.ts` (+87 -0) 📝 `src/pcap/PcapOutput.ts` (+8 -8) </details> ### 📄 Description ## Summary Fixes #5734. A valid TShark `frame.protocols: [""]` currently discards the entire capture summary. Keep those frames in the packet count, retain other frames' facts, and omit empty protocol names from the distribution. ## Change and scope Validate the single protocol-stack value once in `PcapOutput` and reuse it during aggregation, removing the redundant per-frame `Set`. CLI, MCP summaries, retained pages and full Evidence share this producer. Missing/multivalued fields, malformed output and invalid frame identities remain refused. Document the distinction between examined frame coverage and observed protocol names. Existing static saved-capture inspection only; additional maintainer scope decision is not applicable. No new schema, authority, cache or resource-limit changes. ## Evidence and validation - `npm run test:focused -- src/pcap/PcapOutput.test.ts src/pcap/PcapConfiguration.test.ts tests/boundary/pcap/pcapProvider.test.ts tests/boundary/pcap/pcapRetainedView.test.ts tests/boundary/mcp/pcapMcp.test.ts tests/boundary/cli/pcapCli.test.ts` — initial 85 tests passed. After review, `npm run test:local -- src/pcap/PcapOutput.test.ts` passes all 27 cases, including two additional endpoint/DNS preservation cases. The updated producer suite fails 6 cases on original main (`335c042d1`). - `npm run check` — passed typecheck, lint, format, knip, metadata and architecture guards. - `npm run docs:check` — passed, including 5 documentation tests. - Real Linux TShark/capinfos 4.0.17, public CLI and SDK stdio MCP: empty capture; all-zero-length, nonempty and mixed user0 captures; 20,000 mixed records; and the unchanged 43-frame [Wireshark HTTP sample](https://wiki.wireshark.org/SampleCaptures). All six pass after the fix. The three affected inputs fail with `unreadable_output` on original main. - Example restored result: two zero-length records → `packet_count: 2`, `protocol_count: 0`, `protocol_distribution: []`, `coverage: "complete"`; mixed two-record capture → one `user_dlt:data` observation. Native TShark fields and capinfos counts provide the independent oracle. Repeated summaries preserve parent identity; protocol pages and exported full Evidence match the CLI, target digests remain unchanged, and temporary roots are empty after close. - Review follow-up: native `mergecap` combines the HTTP sample and zero-length user0 records into a 45-frame pcapng. Actual CLI/MCP preserve all four endpoints, two HTTP URIs and the complete five-stack distribution; the native oracle, retained pages and full Evidence export agree. This additionally verifies the mixed real-traffic case, beyond the parser regressions with empty frames before/after a DNS response. ## Performance and limits Equivalent completed HTTP task (catalog, CLI, two MCP summary/page requests, full Evidence export and cleanup), one CPU: before/after **21.84/21.32 s elapsed**, **21.33/20.95 s CPU**, **1219.73/1271.83 MiB peak process-family memory**. Fresh processes; OS caches uncontrolled. First MCP summary/page: 10.49/9.96 s; repeat: 1.59/1.28 s. This single pair establishes no speedup or memory reduction. Catalogs are identical. Captured JSON-RPC cl100k_base estimates are equal at 894,123 tokens including the full catalog (884,676), arguments, replies and repeats; the incoming initialization reply and actual model usage are unmeasured. Input-only selected-tool schema estimate is 14,735 tokens. These are serialization estimates, not claimed agent token consumption or savings. All qualified scopes use 8 GiB memory, a 7 GiB early-stop threshold, no swap and one CPU. Payload-exit cgroup receipts show zero OOM events and only the accounting observer remaining. Native Windows/macOS and other Wireshark versions remain unverified. No breaking change or package metadata change. ## Author readiness Human owner: @gensokyo-dev The session's human owner explicitly delegated completion of these declarations and requested initial ready-for-review publication; this records that standing instruction, without claiming an additional manual human review occurred during this run. - [x] I reviewed the final diff and relevant existing code, understand the change and its edge cases, and take responsibility for the complete PR, including AI-assisted work. - [x] I checked the evidence above, confirmed it supports the current change, and recorded meaningful validation gaps. - [x] I declare this PR ready for initial maintainer review. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-11-01 21:44:55 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-11-03 13:39:01 +00:00

[PR #7043] [MERGED] fix(browser): admit passive capture events against the shared retained budget #7599

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

morluto/rea#7043

No description provided.

Rows
Columns