morluto/rea

Fork 0

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

[PR #4901] [CLOSED] fix(process): retain kernel limit descriptor cleanup capability #6667

New issue

Closed

opened 2026-11-01 21:42:10 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-11-01 21:42:10 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/4901
Author: @instrutorclaudiobastos-sudo
Created: 10/17/2026
Status: ❌ Closed

Base: main ← Head: fix/linux-kernel-descriptor-cleanup

📄 Description

Summary

Preserve retryable ownership when closing /proc/self/limits fails. The shared artifact-file owner now carries the exact cleanup capability through the error boundary, so a later request or provider shutdown can retry the same descriptor instead of retaining an unrecoverable placeholder.

Problem and expected behavior

Four Linux descriptor-cleanup cases failed in CI at f5fcf5216: capture and signal operations, including field/decoder discovery. The kernel-limit reader retained the descriptor in its scope but projected only a generic cleanup observation. ArtifactAnalysisResources consequently could not recover its cleanup lease. No separate issue was opened; this PR addresses that executed CI failure.

Change and scope

Replace the reader's manual close/error projection with withArtifactFileHandle, already used by other artifact readers. Remove the superseded cleanup path. Add cross-platform boundary regressions using real temporary-file descriptors for both valid and malformed reports, including failed retry and successful recovery.

Contract and boundary impact

Owner: kernel-report descriptor acquisition and final close.
Scope decision: not applicable; routine ownership fix within existing read-only analysis boundaries.
CLI/MCP, Evidence provenance, package metadata and installation contracts: unchanged.
Platform: Linux kernel-limit reading; no additional target selection, execution or mutation authority.
Effects: reads the existing kernel report and releases its owned descriptor; does not write target bytes.

Evidence and regression coverage

Both new cases failed on the unmodified base because the failure lacked ArtifactFileCleanupFailure and its exact lease.
With the fix, 29 focused tests passed across the new regression, limit parsing and ArtifactAnalysisResources; five additional ArtifactResourceScope tests passed.
This reproduces the ownership boundary on Windows using a mapped report file, not a real Linux /proc or real tshark/sigrok execution. Linux CI is still required.

Validation performed

Executed before rebasing onto 19329e43b:

npm run check:fast -- --concurrency=1 — passed.
npm run build:cached — passed.
Focused source tests above — 34 passed.
npm run verify:test-discovery — all 1557 files discovered exactly once.

Current rebased revision 8c431467a: local npm run check with TURBO_CONCURRENCY=1 passed types, format, metadata and compilation, then Knip failed with RangeError: Array buffer allocation failed on this 6.2 GiB Windows host. Architecture guards are running separately. The fork CI run has passed static checks and package build; Linux tests are in progress. Upstream PR workflows require maintainer approval for this external contribution.

Compatibility, safety, and release

No public schema or migration change. The original read failure and report path remain attached when close fails. Real Linux/provider coverage remains pending; local Windows tests do not establish it. No release is requested.

Author readiness

Human owner: @instrutorclaudiobastos-sudo (PR author; readiness has not been declared).

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.

Before merge

Human ownership/readiness and current required checks are complete.
Linux descriptor-cleanup coverage is verified on this revision.

_{🔄 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/4901 **Author:** [@instrutorclaudiobastos-sudo](https://github.com/instrutorclaudiobastos-sudo) **Created:** 10/17/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/linux-kernel-descriptor-cleanup` --- ### 📄 Description ## Summary Preserve retryable ownership when closing `/proc/self/limits` fails. The shared artifact-file owner now carries the exact cleanup capability through the error boundary, so a later request or provider shutdown can retry the same descriptor instead of retaining an unrecoverable placeholder. ## Problem and expected behavior Four Linux descriptor-cleanup cases failed in [CI at f5fcf5216](https://github.com/morluto/rea/actions/runs/39067439791/job/117499757532): capture and signal operations, including field/decoder discovery. The kernel-limit reader retained the descriptor in its scope but projected only a generic cleanup observation. `ArtifactAnalysisResources` consequently could not recover its cleanup lease. No separate issue was opened; this PR addresses that executed CI failure. ## Change and scope Replace the reader's manual close/error projection with `withArtifactFileHandle`, already used by other artifact readers. Remove the superseded cleanup path. Add cross-platform boundary regressions using real temporary-file descriptors for both valid and malformed reports, including failed retry and successful recovery. ## Contract and boundary impact - Owner: kernel-report descriptor acquisition and final close. - Scope decision: not applicable; routine ownership fix within existing read-only analysis boundaries. - CLI/MCP, Evidence provenance, package metadata and installation contracts: unchanged. - Platform: Linux kernel-limit reading; no additional target selection, execution or mutation authority. - Effects: reads the existing kernel report and releases its owned descriptor; does not write target bytes. ## Evidence and regression coverage - Both new cases failed on the unmodified base because the failure lacked `ArtifactFileCleanupFailure` and its exact lease. - With the fix, 29 focused tests passed across the new regression, limit parsing and `ArtifactAnalysisResources`; five additional `ArtifactResourceScope` tests passed. - This reproduces the ownership boundary on Windows using a mapped report file, not a real Linux `/proc` or real tshark/sigrok execution. Linux CI is still required. ## Validation performed Executed before rebasing onto `19329e43b`: - `npm run check:fast -- --concurrency=1` — passed. - `npm run build:cached` — passed. - Focused source tests above — 34 passed. - `npm run verify:test-discovery` — all 1557 files discovered exactly once. Current rebased revision `8c431467a`: local `npm run check` with `TURBO_CONCURRENCY=1` passed types, format, metadata and compilation, then Knip failed with `RangeError: Array buffer allocation failed` on this 6.2 GiB Windows host. Architecture guards are running separately. The [fork CI run](https://github.com/instrutorclaudiobastos-sudo/rea/actions/runs/39075331694) has passed static checks and package build; Linux tests are in progress. Upstream PR workflows require maintainer approval for this external contribution. ## Compatibility, safety, and release No public schema or migration change. The original read failure and report path remain attached when close fails. Real Linux/provider coverage remains pending; local Windows tests do not establish it. No release is requested. ## Author readiness Human owner: @instrutorclaudiobastos-sudo (PR author; readiness has not been declared). - [ ] 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. ## Before merge - [ ] Human ownership/readiness and current required checks are complete. - [ ] Linux descriptor-cleanup coverage is verified on this revision. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-11-01 21:42:10 +00:00

closed this issue
added the
pull-request
label

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#6667

No description provided.

Rows
Columns