morluto/rea

Fork 0

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

[GH-ISSUE #4888] [Bug] Update subprocess output is retained without a resource bound #3036

New issue

Closed

opened 2026-10-31 00:35:13 +00:00 by bsakaguchi · 3 comments

bsakaguchi commented

2026-10-31 00:35:13 +00:00

Owner

Originally created by @morluto on GitHub (Oct 17, 2026).
Original GitHub issue: https://github.com/morluto/rea/issues/4888

Before submitting

Searched open and closed reports and current UpdateRuntime PRs.
Source-only report; no private data or credentials included.

Area

Setup, packaging, or upgrade

Observed behavior

Source-derived at c555caad6558e801e99155cb199ba12d11aa7826: runUpdateCommand retains every stdout/stderr chunk with stdout += chunk and stderr += chunk, without a collection limit. Human mode streams the chunks but still retains the full strings. Both registry/installation probes and package installation use this collector.

Evidence: owning collector, production callers.

Reproduction

Source inspection only. Read the two data listeners and their callers. No memory-exhausting subprocess was run. NOT_ENOUGH_INFO for an observed OOM, output threshold or failure frequency; none is claimed.

Expected behavior

Keep package-manager collection within an explicit resource budget. Preserve complete small metadata and meaningful bounded diagnostics, drain supervised children, and report an explicit capacity failure when a required complete reply exceeds that budget. Do not silently parse truncated registry metadata as success. Reuse process supervision/cleanup ownership rather than adding an unrelated capture mechanism.

Impact and scope

Source-derived: a noisy or non-terminating package-manager process can grow REA's retained strings without a resource bound. Human output streaming does not remove that retention. Actual peak memory, affected output sizes and runtime frequency are NOT_ENOUGH_INFO. No provider or target execution was investigated.

REA version

6.5.0 source at c555caad65.

Runtime and platform

Source review on macOS arm64 with Node24.18.0. Native Windows/Linux and exhausting output are NOT RUN.

Target and artifact context

Package-manager stdout/stderr; no reverse-engineering target.

Interface

CLI, rea update subprocess collection.

Earliest failure stage

Package-manager command output collection.

Typed outcome or recovery expected

A capacity outcome and bounded diagnostics with owned child cleanup, retaining relevant command/exit facts. Exact budget/API selection remains to be designed.

Additional context

Distinct from #4883's Incur updater dispatch/Windows shim bug. Routing that flag through the existing verified updater does not itself add or remove collection bounds. PR3557 concerns optional native module readiness rather than this collector's resource policy.

Originally created by @morluto on GitHub (Oct 17, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/4888 ### Before submitting - [x] Searched open and closed reports and current UpdateRuntime PRs. - [x] Source-only report; no private data or credentials included. ### Area Setup, packaging, or upgrade ### Observed behavior Source-derived at `c555caad6558e801e99155cb199ba12d11aa7826`: `runUpdateCommand` retains every stdout/stderr chunk with `stdout += chunk` and `stderr += chunk`, without a collection limit. Human mode streams the chunks but still retains the full strings. Both registry/installation probes and package installation use this collector. Evidence: [owning collector](https://github.com/morluto/rea/blob/c555caad6558e801e99155cb199ba12d11aa7826/src/application/UpdateRuntime.ts#L38), [production callers](https://github.com/morluto/rea/blob/c555caad6558e801e99155cb199ba12d11aa7826/src/application/UpdateRuntime.ts#L299). ### Reproduction Source inspection only. Read the two data listeners and their callers. No memory-exhausting subprocess was run. NOT_ENOUGH_INFO for an observed OOM, output threshold or failure frequency; none is claimed. ### Expected behavior Keep package-manager collection within an explicit resource budget. Preserve complete small metadata and meaningful bounded diagnostics, drain supervised children, and report an explicit capacity failure when a required complete reply exceeds that budget. Do not silently parse truncated registry metadata as success. Reuse process supervision/cleanup ownership rather than adding an unrelated capture mechanism. ### Impact and scope Source-derived: a noisy or non-terminating package-manager process can grow REA's retained strings without a resource bound. Human output streaming does not remove that retention. Actual peak memory, affected output sizes and runtime frequency are NOT_ENOUGH_INFO. No provider or target execution was investigated. ### REA version 6.5.0 source at c555caad6558e801e99155cb199ba12d11aa7826. ### Runtime and platform Source review on macOS arm64 with Node24.18.0. Native Windows/Linux and exhausting output are NOT RUN. ### Target and artifact context Package-manager stdout/stderr; no reverse-engineering target. ### Interface CLI, `rea update` subprocess collection. ### Earliest failure stage Package-manager command output collection. ### Typed outcome or recovery expected A capacity outcome and bounded diagnostics with owned child cleanup, retaining relevant command/exit facts. Exact budget/API selection remains to be designed. ### Additional context Distinct from #4883's Incur updater dispatch/Windows shim bug. Routing that flag through the existing verified updater does not itself add or remove collection bounds. PR3557 concerns optional native module readiness rather than this collector's resource policy.

bsakaguchi

2026-10-31 00:35:13 +00:00

closed this issue
added the
triage

bug
labels

bsakaguchi commented

2026-10-31 00:35:14 +00:00

Author

Owner

@PrafulK99 commented on GitHub (Oct 17, 2026):

I would like to take a look at this and work on a fix. I will investigate the output handling and add tests to ensure we bound memory usage without breaking the update flow.

@PrafulK99 commented on GitHub (Oct 17, 2026): I would like to take a look at this and work on a fix. I will investigate the output handling and add tests to ensure we bound memory usage without breaking the update flow.

bsakaguchi commented

2026-10-31 00:35:14 +00:00

Author

Owner

@morluto commented on GitHub (Oct 17, 2026):

Fixed on main in 182e9a268 and 01cdbbdc5.

The producing collector now uses a shared 4 MiB aggregate raw-byte budget, drains commands through physical stream closure (including after an early process error), and rejects oversized required metadata. Verbose installs finish normally; failed installs explicitly mark omitted diagnostics. Human progress owns backpressure and submitted-write acknowledgements without ending borrowed stderr. Existing command resolution, selected inputs, runtime prerequisites and signal/tree policy are preserved.

Validation: 154 distinct focused checks across 12 files (152-check full run plus two added waiter controls in a 44-check rerun), static checks, documentation checks, test discovery (1563 files), pre-push checks, and two source reviews pass. The public oversized-metadata regression fails on 4f7b5fb60 and passes the fix. Regression and fake Bun workflows exercise complete replies, UTF-8, early error/late close, noisy install success/failure, and delayed progress-write failures.

Small cost comparison: six sequential completed public current-version checks with owned rejecting fake npm; first-process observations separate from two alternating warm repeats. Median elapsed 0.400→0.404s, CPU 0.602→0.622s, sampled family RSS 384.2→353.5 MB; identical replies and estimated request/reply/progress tokens 6/28/0. These small samples do not establish speedup, significance, true-cold behavior or large-install memory savings.

Executed on macOS arm64/Node24.18.0 with owned Node and fake package-manager commands. No real installation, registry request, OOM/exhausting-output test, native Windows/Linux run, heavy CPU test or profiler run is claimed. The limit covers retained raw bytes; decoding, snapshots and stream buffers add bounded overhead. Existing updater signal/descendant cleanup behavior remains outside this output-retention fix.

@morluto commented on GitHub (Oct 17, 2026): Fixed on main in 182e9a268 and 01cdbbdc5. The producing collector now uses a shared 4 MiB aggregate raw-byte budget, drains commands through physical stream closure (including after an early process error), and rejects oversized required metadata. Verbose installs finish normally; failed installs explicitly mark omitted diagnostics. Human progress owns backpressure and submitted-write acknowledgements without ending borrowed stderr. Existing command resolution, selected inputs, runtime prerequisites and signal/tree policy are preserved. Validation: 154 distinct focused checks across 12 files (152-check full run plus two added waiter controls in a 44-check rerun), static checks, documentation checks, test discovery (1563 files), pre-push checks, and two source reviews pass. The public oversized-metadata regression fails on 4f7b5fb60 and passes the fix. [Regression and fake Bun workflows](https://github.com/morluto/rea/commit/01cdbbdc5) exercise complete replies, UTF-8, early error/late close, noisy install success/failure, and delayed progress-write failures. Small cost comparison: six sequential completed public current-version checks with owned rejecting fake npm; first-process observations separate from two alternating warm repeats. Median elapsed 0.400→0.404s, CPU 0.602→0.622s, sampled family RSS 384.2→353.5 MB; identical replies and estimated request/reply/progress tokens 6/28/0. These small samples do not establish speedup, significance, true-cold behavior or large-install memory savings. Executed on macOS arm64/Node24.18.0 with owned Node and fake package-manager commands. No real installation, registry request, OOM/exhausting-output test, native Windows/Linux run, heavy CPU test or profiler run is claimed. The limit covers retained raw bytes; decoding, snapshots and stream buffers add bounded overhead. Existing updater signal/descendant cleanup behavior remains outside this output-retention fix.

bsakaguchi commented

2026-10-31 00:35:14 +00:00

Author

Owner

@xu91102 commented on GitHub (Oct 17, 2026):

I reproduced the collection behavior on current main 4f7b5fb6023ce2ec449561782d09e3c7e5ac7c7b using the compiled production updater and finite, self-authored Node subprocesses.

For each of stdout-success and stderr-failure (exit 2), outputs of 65,536, 1,048,576 and 12,582,912 bytes were returned as strings of exactly the same sizes. All six child processes reached their completion marker. A compiled public rea update --json control with a strict local npm fixture also completed as current for both normal metadata and valid version metadata prefixed by 12 MiB of JSON whitespace; only root/view probes ran, with no install command or real registry access.

This demonstrates retained output growing with finite child output, not an observed OOM or a natural failure rate. Node24.18.0/macOS arm64 only; native Windows/Linux, real npm/Bun installation, full agent workflow and performance benefits are NOT RUN.

I read the actual current #3557 production diff: its optional-provider readiness work does not bound runUpdateCommand collection. No upstream PR has been created.

Update after refreshing main: 182e9a268 and 01cdbbdc556262c47d04a5b3391dd9c17946d3fc already fix this shared collection root, and the maintainer closed this issue at 21:58:39Z, before this reproduction comment was posted. I read both actual production diffs. I stopped before creating a task worktree or making any implementation/commit/push; there is no patch or PR from me to deliver or attribute.

I rebuilt exact main 01cdbbdc556262c47d04a5b3391dd9c17946d3fc and reran my six finite child controls plus two public CLI controls using the same unchanged child/npm fixture bytes. The two 12 MiB command replies now return the explicit complete-output capacity error (105 bytes); both smaller outputs retain their previous exact results. All six finite children complete. The public normal-metadata control remains current, while oversized metadata returns failed/version-check with the capacity explanation and exit 1. No install command or real registry access occurred. This verifies only these eight controlled macOS cases, not the full main test suite, real installations, Windows/Linux, OOM behavior or performance. I will not submit a duplicate fix for this root.

@xu91102 commented on GitHub (Oct 17, 2026): I reproduced the collection behavior on current main `4f7b5fb6023ce2ec449561782d09e3c7e5ac7c7b` using the compiled production updater and finite, self-authored Node subprocesses. For each of stdout-success and stderr-failure (exit 2), outputs of 65,536, 1,048,576 and 12,582,912 bytes were returned as strings of exactly the same sizes. All six child processes reached their completion marker. A compiled public `rea update --json` control with a strict local npm fixture also completed as `current` for both normal metadata and valid version metadata prefixed by 12 MiB of JSON whitespace; only `root`/`view` probes ran, with no install command or real registry access. This demonstrates retained output growing with finite child output, not an observed OOM or a natural failure rate. Node24.18.0/macOS arm64 only; native Windows/Linux, real npm/Bun installation, full agent workflow and performance benefits are NOT RUN. I read the actual current #3557 production diff: its optional-provider readiness work does not bound `runUpdateCommand` collection. No upstream PR has been created. Update after refreshing main: `182e9a268` and `01cdbbdc556262c47d04a5b3391dd9c17946d3fc` already fix this shared collection root, and the maintainer closed this issue at 21:58:39Z, before this reproduction comment was posted. I read both actual production diffs. I stopped before creating a task worktree or making any implementation/commit/push; there is no patch or PR from me to deliver or attribute. I rebuilt exact main `01cdbbdc556262c47d04a5b3391dd9c17946d3fc` and reran my six finite child controls plus two public CLI controls using the same unchanged child/npm fixture bytes. The two 12 MiB command replies now return the explicit complete-output capacity error (105 bytes); both smaller outputs retain their previous exact results. All six finite children complete. The public normal-metadata control remains `current`, while oversized metadata returns `failed`/`version-check` with the capacity explanation and exit 1. No install command or real registry access occurred. This verifies only these eight controlled macOS cases, not the full main test suite, real installations, Windows/Linux, OOM behavior or performance. I will not submit a duplicate fix for this root.

bsakaguchi referenced this issue

2026-10-31 01:07:41 +00:00

[PR #3036] [CLOSED] fix(javascript): keep keyed-store reads out of endpoints #5572

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

No description provided.

Rows
Columns