morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 06:45:22 +00:00

[PR #2284] [MERGED] fix: preserve process identities in pwntools cleanup errors #5140

New issue

Closed

opened 2026-10-31 01:04:16 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:04:16 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/2284
Author: @jeonjw85
Created: 10/12/2026
Status: ✅ Merged
Merged: 10/13/2026
Merged by: @morluto

Base: main ← Head: fix/pwntools-cleanup-resources

📝 Commits (2)

818c72c fix: preserve process identities in pwntools cleanup errors
54a9d82 fix(pwntools): integrate current cleanup ownership contracts

📊 Changes

4 files changed (+45 additions, -31 deletions)

View changed files

📝 src/native/pwntools/PwntoolsDecoder.ts (+11 -12)
📝 src/process/OwnedCommand.ts (+13 -6)
📝 tests/boundary/process/ownedCommand.test.ts (+5 -3)
📝 tests/process-global/pwntoolsStageRetirement.test.ts (+16 -10)

📄 Description

Summary

Fixes #2274.

Include the recorded leader PID and process-group ID when a pwntools decoder or debugger retains a process after incomplete cleanup.

Problem and expected behavior

The provider retained the supervisor but reported only its run ID and runtime directory. A caller inspecting cleanup_incomplete could not identify the retained process group. The direct OwnedCommand failure path already reported those identifiers.

Change and scope

Both paths now use ownedCommandResources to format the recorded launch ownership. The decoder continues to retain its supervisor and runtime until cleanup succeeds.

This is a diagnostic fix within the existing provider boundary; no new capability or maintainer scope decision is needed.

Contract and boundary impact

Owner and stage: owned-command resource reporting during failed cleanup.
CLI/MCP: details.resources gains pid:<leaderPid> and process-group:<processGroupId> entries. No schema or catalog change.
Evidence: original errors, captured output, partial observations, run ID, and runtime path remain available.
Authority: identifiers describe the recorded launch. They do not establish current liveness or grant permission to signal a process.
Provider/platform and packaging: no admission, launcher, dependency, installation, or generated-metadata changes.

Evidence and regression coverage

The existing stage-retirement tests now assert the exact projected resources for decoder and debugger stages, after both successful and failed commands. Those four cases failed on the base revision because the PID/group entries were missing. The direct owned-command tests also check both identifiers.

Representative resource list:

["rea-artifact-decoder-<run-id>", "/path/to/owned-runtime", "pid:1234", "process-group:1234"]

Validation performed

Using Node 24.18.0 and npm 11.16.0 on macOS:

npm run test:focused -- tests/process-global/pwntoolsStageRetirement.test.ts tests/boundary/process/ownedCommand.test.ts — 20 passed.
npm run check — passed, with no lint errors.
git diff --check — passed.

The process tests use real macOS child processes with injected decoder launchers. I did not run the real Linux pwntools/pwndbg lane or native Windows verification.

Compatibility and release

Additive error diagnostics; no migration or release metadata changes.

Before merge

One focused fix, linked to the issue.
Existing scope and cleanup authority preserved.
Regression tests cover the changed observable output.
Final diff checked for unrelated changes and credentials.

_{🔄 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/2284 **Author:** [@jeonjw85](https://github.com/jeonjw85) **Created:** 10/12/2026 **Status:** ✅ Merged **Merged:** 10/13/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `fix/pwntools-cleanup-resources` --- ### 📝 Commits (2) - [`818c72c`](https://github.com/morluto/rea/commit/818c72cc2b2f1571e6b5db43a99b8496e94d2449) fix: preserve process identities in pwntools cleanup errors - [`54a9d82`](https://github.com/morluto/rea/commit/54a9d82e14a24659f38210204ec90a345d3dabdf) fix(pwntools): integrate current cleanup ownership contracts ### 📊 Changes **4 files changed** (+45 additions, -31 deletions) <details> <summary>View changed files</summary> 📝 `src/native/pwntools/PwntoolsDecoder.ts` (+11 -12) 📝 `src/process/OwnedCommand.ts` (+13 -6) 📝 `tests/boundary/process/ownedCommand.test.ts` (+5 -3) 📝 `tests/process-global/pwntoolsStageRetirement.test.ts` (+16 -10) </details> ### 📄 Description ## Summary Fixes #2274. Include the recorded leader PID and process-group ID when a pwntools decoder or debugger retains a process after incomplete cleanup. ## Problem and expected behavior The provider retained the supervisor but reported only its run ID and runtime directory. A caller inspecting `cleanup_incomplete` could not identify the retained process group. The direct `OwnedCommand` failure path already reported those identifiers. ## Change and scope Both paths now use `ownedCommandResources` to format the recorded launch ownership. The decoder continues to retain its supervisor and runtime until cleanup succeeds. This is a diagnostic fix within the existing provider boundary; no new capability or maintainer scope decision is needed. ## Contract and boundary impact - Owner and stage: owned-command resource reporting during failed cleanup. - CLI/MCP: `details.resources` gains `pid:<leaderPid>` and `process-group:<processGroupId>` entries. No schema or catalog change. - Evidence: original errors, captured output, partial observations, run ID, and runtime path remain available. - Authority: identifiers describe the recorded launch. They do not establish current liveness or grant permission to signal a process. - Provider/platform and packaging: no admission, launcher, dependency, installation, or generated-metadata changes. ## Evidence and regression coverage The existing stage-retirement tests now assert the exact projected resources for decoder and debugger stages, after both successful and failed commands. Those four cases failed on the base revision because the PID/group entries were missing. The direct owned-command tests also check both identifiers. Representative resource list: ```json ["rea-artifact-decoder-<run-id>", "/path/to/owned-runtime", "pid:1234", "process-group:1234"] ``` ## Validation performed Using Node 24.18.0 and npm 11.16.0 on macOS: - `npm run test:focused -- tests/process-global/pwntoolsStageRetirement.test.ts tests/boundary/process/ownedCommand.test.ts` — 20 passed. - `npm run check` — passed, with no lint errors. - `git diff --check` — passed. The process tests use real macOS child processes with injected decoder launchers. I did not run the real Linux pwntools/pwndbg lane or native Windows verification. ## Compatibility and release Additive error diagnostics; no migration or release metadata changes. ## Before merge - [x] One focused fix, linked to the issue. - [x] Existing scope and cleanup authority preserved. - [x] Regression tests cover the changed observable output. - [x] Final diff checked for unrelated changes and credentials. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 01:04:16 +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#5140

No description provided.

Rows
Columns