[PR #2698] [MERGED] refactor(process): give the prlimit producer budget one owner #5323

New issue

Closed

opened 2026-10-31 01:05:44 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:05:44 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/2698
Author: @NIU-123370
Created: 10/13/2026
Status: ✅ Merged
Merged: 10/13/2026
Merged by: @morluto

Base: main ← Head: feat/prlimit-producer-limits-owner

📝 Commits (2)

6eded18 refactor(process): give the prlimit producer budget one owner
c2eb5c3 chore(ci): merge current main verification fixes

📊 Changes

5 files changed (+93 additions, -35 deletions)

View changed files

📝 scripts/verify/architecture-guards.mjs (+24 -0)
📝 src/captures/TsharkCaptureProvider.ts (+3 -17)
📝 src/process/LinuxResourceLimits.test.ts (+40 -1)
📝 src/process/LinuxResourceLimits.ts (+23 -0)
📝 src/signals/SigrokRecordingProvider.ts (+3 -17)

📄 Description

Summary

Two providers landed on main three minutes apart on 2026-10-13 and each brought its own copy of the same budget — 32 MiB resident input, 4 MiB retained output, a 30 s command deadline, and the 1 GiB / 64 MiB / 30 CPU s prlimit ceilings — plus its own copy of the prlimit argv that pins them. This moves both to one owner in src/process/LinuxResourceLimits.ts, the module that already owns LinuxResourceLimits, clampLinuxResourceLimits and readLinuxResourceLimits, and adds the census rule that keeps them there.

No value changes. Every number, every option string and the ordering of the vector are identical; the two lanes behave exactly as before.

Where the duplication came from

git blame on the first line of each new clone pair (16-13 07:1xZ): 9c34c091 feat(captures): discover installed protocol and field definitions (#2496) accounts for 21 hits and 4276aaf1 feat(signals): discover installed decoders and option help (#2487) for 19. The pair that matters here is 15 clones between src/captures/TsharkCaptureProvider.ts and src/signals/SigrokRecordingProvider.ts, of which the budget block (15 lines) and the limiter vector (13 lines) are the two that describe one policy rather than two features.

Checked before touching it: neither file carries a mirror/"keep in sync" comment, and no test imports both providers, so nothing in the repo had already decided the copy was intentional.

Change

src/process/LinuxResourceLimits.ts — exports BOUNDED_PRODUCER_LIMITS (the six-field budget) and prlimitArgumentVector(limits, command) (soft and hard pinned to the same value, core dumps disabled, -- then the producer).
src/captures/TsharkCaptureProvider.ts, src/signals/SigrokRecordingProvider.ts — import the budget as LIMITS, delete the local copies, and build the limiter vector through the owner. 17 lines removed and 3 added in each.
scripts/verify/architecture-guards.mjs — two census rules in the existing check(label, files, pattern, allow) style, one per invariant.
src/process/LinuxResourceLimits.test.ts — pins the budget's six values and the exact vector, including the vector after clampLinuxResourceLimits narrows one field.

Audited, deliberately out of scope

Four other lanes spell prlimit options. They were read before writing the rules, and they do not share this budget:

lane	why it stays
`src/pcap/PcapNativeBound.ts`	pairs soft and hard like the owners, but budgets `cpu_seconds: 300` and has no resident-input/output ceiling
`src/evm/EvmoleInterfaceProvider.ts`	soft-only form (`--core=0:`, trailing colon on every pair)
`src/firmware/FirmwareCommand.ts`	soft-only (`--core=0`), `--cpu=120`, and an output budget taken from the call context
`src/javascript/recovery/WakaruCommand.ts`	soft-only, `--cpu=120`

The paired-vector rule allow-lists the pcap lane explicitly rather than leaving it to a future reader. Merging these four into the owner means deciding whether 300/120 CPU seconds and soft-only pinning are one policy or three, which is a behaviour question for whoever owns those lanes — worth a follow-up, not part of this diff.

Contract and scope impact

Public API: two new exports from src/process/LinuxResourceLimits.js; nothing removed or renamed. Provider identities, tool schemas, MCP catalog, docs and error codes are untouched — no generated artifact needs regenerating.
Behaviour: identical. The vectors are produced from the same values in the same order, so the kernel-visible limits for inspect_packet_capture and decode_signal_recording do not move.
Security/containment: the budget is the containment policy for those two lanes; this PR makes it one place to audit. It does not widen any ceiling (1 GiB address space, 64 MiB per file, 30 CPU seconds, no core dumps, unchanged).
Platform: both lanes remain Linux + util-linux prlimit only.

Evidence

Local, on 6eded185 against bf34d0d2:

tsc --noEmit — exit 0, no output.
oxfmt --check on all five files — All matched files use the correct format.
node scripts/ci/static-check-report.mjs lint — exit 0 with the identical diagnostic set to bf34d0d2 (eslint(no-unsafe-finally): 14, unicorn(no-useless-spread): 5), so no new lint findings.
knip — silent.
node scripts/verify/architecture-guards.mjs — Architecture guards passed across 1297 source files.
vitest run tests/boundary/captures/ tests/boundary/signals/ src/process/ — 295 passed | 14 skipped; the skips are the it.skipIf(process.platform !== "linux") lanes in resourceLimits.test.ts, which assert the same numbers (1073741824/67108864/RLIMIT_CORE == (0, 0)) through real prlimit, so Linux CI is what exercises them.

Mutation matrix — nine runs (two baselines and seven mutations), each fed through the real gate rather than reasoned:

mutation	expected	result
baseline, no mutation (unit run and guard run)	green	green, green
`cpu_seconds: 30 -> 31` in the owner	unit test red	red
owner drops `--core=0:0`	unit test red	red
owner emits `--as=<soft>` without the hard half	unit test red	red
owner export renamed away from the providers	red	red
tshark re-inlines the limiter vector	guard red	red, `inline paired prlimit ceilings`
sigrok re-inlines the whole budget	guard red	red, `inline resident-input budget`
sigrok re-inlines only the three ceilings, no input budget	guard green	green

The last row is the rule's real limit: the census matches shapes, like the other rules in that file, so a lane that writes 1024 ** 3 or camelCase fields is not caught. That is stated in the comment above the rule.

About the duplication gate

This PR takes the repository duplication from 1480 clones, 3.0096991% to 1478 clones, 3.0045541% (npm run jscpd, same 2367 sources), so it does not by itself clear the 3.0% gate — main is over it on its own. #2691 is the gate-focused sibling (it shares repeated boundary-test preambles); the two changes touch no common file.

Test plan

Test shard lanes on Linux run tests/boundary/captures/resourceLimits.test.ts and tests/boundary/signals/resourceLimits.test.ts — these launch a real producer under prlimit and assert the address-space and file-size ceilings, so they fail loudly if the owner's vector is wrong.
Static checks runs the new architecture-guard rules over all of src.
src/process/LinuxResourceLimits.test.ts passes on every host (no platform gate).

_{🔄 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/2698 **Author:** [@NIU-123370](https://github.com/NIU-123370) **Created:** 10/13/2026 **Status:** ✅ Merged **Merged:** 10/13/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `feat/prlimit-producer-limits-owner` --- ### 📝 Commits (2) - [`6eded18`](https://github.com/morluto/rea/commit/6eded1859651df3a218bb8e75bc11d2eb58aff79) refactor(process): give the prlimit producer budget one owner - [`c2eb5c3`](https://github.com/morluto/rea/commit/c2eb5c30639a26fe03ce1c3ad04da88f5f322d61) chore(ci): merge current main verification fixes ### 📊 Changes **5 files changed** (+93 additions, -35 deletions) <details> <summary>View changed files</summary> 📝 `scripts/verify/architecture-guards.mjs` (+24 -0) 📝 `src/captures/TsharkCaptureProvider.ts` (+3 -17) 📝 `src/process/LinuxResourceLimits.test.ts` (+40 -1) 📝 `src/process/LinuxResourceLimits.ts` (+23 -0) 📝 `src/signals/SigrokRecordingProvider.ts` (+3 -17) </details> ### 📄 Description ## Summary Two providers landed on `main` three minutes apart on 2026-10-13 and each brought its own copy of the same budget — 32 MiB resident input, 4 MiB retained output, a 30 s command deadline, and the `1 GiB / 64 MiB / 30 CPU s` prlimit ceilings — plus its own copy of the prlimit argv that pins them. This moves both to one owner in `src/process/LinuxResourceLimits.ts`, the module that already owns `LinuxResourceLimits`, `clampLinuxResourceLimits` and `readLinuxResourceLimits`, and adds the census rule that keeps them there. No value changes. Every number, every option string and the ordering of the vector are identical; the two lanes behave exactly as before. ## Where the duplication came from `git blame` on the first line of each new clone pair (16-13 07:1xZ): `9c34c091 feat(captures): discover installed protocol and field definitions (#2496)` accounts for 21 hits and `4276aaf1 feat(signals): discover installed decoders and option help (#2487)` for 19. The pair that matters here is 15 clones between `src/captures/TsharkCaptureProvider.ts` and `src/signals/SigrokRecordingProvider.ts`, of which the budget block (15 lines) and the limiter vector (13 lines) are the two that describe one policy rather than two features. Checked before touching it: neither file carries a mirror/"keep in sync" comment, and no test imports both providers, so nothing in the repo had already decided the copy was intentional. ## Change - `src/process/LinuxResourceLimits.ts` — exports `BOUNDED_PRODUCER_LIMITS` (the six-field budget) and `prlimitArgumentVector(limits, command)` (soft and hard pinned to the same value, core dumps disabled, `--` then the producer). - `src/captures/TsharkCaptureProvider.ts`, `src/signals/SigrokRecordingProvider.ts` — import the budget as `LIMITS`, delete the local copies, and build the limiter vector through the owner. 17 lines removed and 3 added in each. - `scripts/verify/architecture-guards.mjs` — two census rules in the existing `check(label, files, pattern, allow)` style, one per invariant. - `src/process/LinuxResourceLimits.test.ts` — pins the budget's six values and the exact vector, including the vector after `clampLinuxResourceLimits` narrows one field. ## Audited, deliberately out of scope Four other lanes spell prlimit options. They were read before writing the rules, and they do not share this budget: | lane | why it stays | | --- | --- | | `src/pcap/PcapNativeBound.ts` | pairs soft and hard like the owners, but budgets `cpu_seconds: 300` and has no resident-input/output ceiling | | `src/evm/EvmoleInterfaceProvider.ts` | soft-only form (`--core=0:`, trailing colon on every pair) | | `src/firmware/FirmwareCommand.ts` | soft-only (`--core=0`), `--cpu=120`, and an output budget taken from the call context | | `src/javascript/recovery/WakaruCommand.ts` | soft-only, `--cpu=120` | The paired-vector rule allow-lists the pcap lane explicitly rather than leaving it to a future reader. Merging these four into the owner means deciding whether 300/120 CPU seconds and soft-only pinning are one policy or three, which is a behaviour question for whoever owns those lanes — worth a follow-up, not part of this diff. ## Contract and scope impact - Public API: two new exports from `src/process/LinuxResourceLimits.js`; nothing removed or renamed. Provider identities, tool schemas, MCP catalog, docs and error codes are untouched — no generated artifact needs regenerating. - Behaviour: identical. The vectors are produced from the same values in the same order, so the kernel-visible limits for `inspect_packet_capture` and `decode_signal_recording` do not move. - Security/containment: the budget is the containment policy for those two lanes; this PR makes it one place to audit. It does not widen any ceiling (`1 GiB` address space, `64 MiB` per file, `30` CPU seconds, no core dumps, unchanged). - Platform: both lanes remain Linux + util-linux prlimit only. ## Evidence Local, on `6eded185` against `bf34d0d2`: - `tsc --noEmit` — exit 0, no output. - `oxfmt --check` on all five files — `All matched files use the correct format.` - `node scripts/ci/static-check-report.mjs lint` — exit 0 with the identical diagnostic set to `bf34d0d2` (`eslint(no-unsafe-finally)`: 14, `unicorn(no-useless-spread)`: 5), so no new lint findings. - `knip` — silent. - `node scripts/verify/architecture-guards.mjs` — `Architecture guards passed across 1297 source files.` - `vitest run tests/boundary/captures/ tests/boundary/signals/ src/process/` — `295 passed | 14 skipped`; the skips are the `it.skipIf(process.platform !== "linux")` lanes in `resourceLimits.test.ts`, which assert the same numbers (`1073741824`/`67108864`/`RLIMIT_CORE == (0, 0)`) through real `prlimit`, so Linux CI is what exercises them. Mutation matrix — nine runs (two baselines and seven mutations), each fed through the real gate rather than reasoned: | mutation | expected | result | | --- | --- | --- | | baseline, no mutation (unit run and guard run) | green | green, green | | `cpu_seconds: 30 -> 31` in the owner | unit test red | red | | owner drops `--core=0:0` | unit test red | red | | owner emits `--as=<soft>` without the hard half | unit test red | red | | owner export renamed away from the providers | red | red | | tshark re-inlines the limiter vector | guard red | red, `inline paired prlimit ceilings` | | sigrok re-inlines the whole budget | guard red | red, `inline resident-input budget` | | sigrok re-inlines only the three ceilings, no input budget | guard green | green | The last row is the rule's real limit: the census matches shapes, like the other rules in that file, so a lane that writes `1024 ** 3` or camelCase fields is not caught. That is stated in the comment above the rule. ## About the duplication gate This PR takes the repository duplication from `1480 clones, 3.0096991%` to `1478 clones, 3.0045541%` (`npm run jscpd`, same 2367 sources), so **it does not by itself clear the 3.0% gate** — `main` is over it on its own. #2691 is the gate-focused sibling (it shares repeated boundary-test preambles); the two changes touch no common file. ## Test plan - [ ] `Test shard` lanes on Linux run `tests/boundary/captures/resourceLimits.test.ts` and `tests/boundary/signals/resourceLimits.test.ts` — these launch a real producer under `prlimit` and assert the address-space and file-size ceilings, so they fail loudly if the owner's vector is wrong. - [ ] `Static checks` runs the new architecture-guard rules over all of `src`. - [ ] `src/process/LinuxResourceLimits.test.ts` passes on every host (no platform gate). --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 01:05:44 +00:00