morluto/rea

Fork 0

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

[PR #5676] [MERGED] perf(test): reuse one compiled CLI process per JavaScript application file #7032

New issue

Closed

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

bsakaguchi commented

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

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/5676
Author: @morluto
Created: 10/20/2026
Status: ✅ Merged
Merged: 10/21/2026
Merged by: @morluto

Base: main ← Head: cursor/reuse-js-cli-session-3c30

📝 Commits (3)

4f64c93 perf(test): reuse one compiled CLI process per JavaScript application file
12e3bdb fix(test): bound shared CLI worker lifetime
9a758be fix(test): retire a shared CLI worker after a transport failure

📊 Changes

8 files changed (+1464 additions, -18 deletions)

View changed files

📝 docs/testing.md (+9 -1)
➕ tests/boundary/cli/sharedJavaScriptCliSession.test.ts (+577 -0)
➕ tests/support/cli/cliCommandSession.ts (+448 -0)
➕ tests/support/cli/cliCommandSessionFrames.d.mts (+13 -0)
➕ tests/support/cli/cliCommandSessionFrames.mjs (+77 -0)
➕ tests/support/cli/cliCommandSessionWorker.mjs (+263 -0)
📝 tests/support/cli/cliFixture.ts (+70 -15)
📝 tests/support/cli/javascriptApplication.ts (+7 -2)

📄 Description

Summary

After stdio MCP reuse, the slow JavaScript application acceptance files were dominated by fresh CLI processes at about 1.4s each. analyzeJavaScriptDirectory now keeps one compiled CLI process per cliTest file for the same child environment. Later CLI cases reuse that process. cli.run still starts a fresh process.

No product issue is linked. This follows the measured gap left by #5658.

Problem and expected behavior

On this host, without coverage, the three slowest files were:

javascriptLiteralPercentPaths.test.ts: 191.9s (96 CLI cases, average 1440ms)
javascriptLoaderReviewBoundaries.test.ts: 120.5s (62 CLI cases, average 1400ms)
javascriptIndirectLoaderOwners.test.ts: 81.4s (41 CLI cases, average 1450ms)

Together they took 205.0s wall with Vitest's two workers. MCP cases in the same files already averaged about 540ms. A warm compile cache was already shared. The remaining CLI time was Node and CLI startup around a roughly 0.3s analysis.

Those CLI cases should keep every assertion and the same child environment as a fresh process, without paying startup on every case.

Change and scope

cliTest owns one file-scoped CLI worker. It loads the compiled runtime once and serves length-prefixed commands. Each command gets the same one-shot argument, environment, and output path as scripts/rea.mjs, including a fresh createCli(). A different environment restarts the worker. The worker also restarts after 32 commands, when it reports a resident set of at least 1 GiB, and after a fatal command failure or a broken session frame. That failure drops the process before the command settles and waits for it to exit before the next command starts. Incomplete session frames stay in separate chunks until one frame is complete, then that frame is joined once. Stdin and explicit deadlines stay on cli.run. File teardown stops the worker, including startup still in flight, and does not open another after close.

The worker is test-only. Product CLI and MCP behavior is unchanged.

Follow the contribution scope.
Unresolved scope, authority, side effects, or required verification block
integration. Link the required maintainer scope decision for new providers or
capabilities involving target execution, injection, mutation, or protection
recovery; ordinary fixes within existing boundaries may state "not applicable".
Draft PRs may leave the scope decision pending and the readiness and merge checklists
unchecked. Maintainers can help resolve open questions.

Contract and boundary impact

Semantic owner and earliest changed stage: test harness. The shared process starts at the first analyzeJavaScriptDirectory CLI call in a cliTest file.
Contribution scope fit and linked maintainer decision: not applicable. Test layout and the existing compiled CLI entry only.
CLI and MCP/tool-catalog contract: none. cli.run and MCP sessions are unchanged.
Provider, bridge, target-format, or platform compatibility: none.
Evidence, artifact, provenance, or reconstruction contract: none. A regression compares normalized_result with a fresh process.
Target selection, authority, upstream behavior, and cleanup details (if applicable): the file fixture owns the worker and stops it on teardown.
Generated metadata (docs/public/product-catalog.json), package, or installation impact (CI generates and validates ignored catalogs and skills/; edit authored instructions in .agents/skills/reverse-engineer-anything/ and never commit build outputs): none.

Expected effects (check known effects; use documentation-only on its own).
Unchecked items do not establish absence; leave uncertainties visible.

Read existing files or captured data.
Launch or attach to a process.
Inject instrumentation or run scripts/code.
Change process memory, registers, files, or configuration.
Contact network services.
Recover protected code or work around protections.
Documentation or usability only; no target interaction.
Not sure yet; investigation or maintainer guidance needed.

The launched process is the test worker. It does not change product launch behavior.

Evidence and regression coverage

Tests added or updated: tests/boundary/cli/sharedJavaScriptCliSession.test.ts covers frame parsing, including a frame delivered one byte at a time, close during startup and in-flight work, output isolation, environment restart, exit recovery, fatal retirement before the next spawn, transport-failure retirement before the next spawn, restart after a command limit and after a reported resident set, byte-level --version and invalid REA_LOG_LEVEL parity with a fresh process, and equal normalized_result across two application directories. analyzeJavaScriptDirectory is included in that parity check.
Base reproduction or other evidence: same three acceptance files, Node 24.18.0, 4 cores, no coverage, one paired run. Process-family CPU is the sum of sampled /proc utime+stime (100 Hz). RSS is the sum of VmRSS and can count shared pages more than once. PSS is from smaps_rollup; 82 of 1023 before-samples and 65 of 580 after-samples had no PSS.
User-visible CLI/MCP output (if applicable): not applicable.
Remaining proof gaps: sibling analyzeJavaScriptDirectory files were not re-timed as a group. One run does not estimate CI variance. Coverage mode was not measured. The worker repeats the scripts/rea.mjs one-shot sequence, so a later entry change is guarded by the fresh-process comparison rather than a shared function. The 1 GiB and 32-command caps were checked with fakes plus six repeats of a tiny Electron fixture; process-family PSS for the original three-file run was not measured again.

	Before	After
Wall	205.0s	116.6s
Process-family CPU	420.7s	207.9s
Peak summed RSS	2740 MiB	4120 MiB
Peak summed PSS	2332 MiB	3815 MiB
Literal file	191.9s	105.8s
Loader file	120.5s	68.4s
Indirect file	81.4s	44.5s
CLI case average	1400–1450ms	530–567ms

All 398 tests passed in both runs. CLI case time now matches the MCP cases in the same files (about 530–570ms). The remaining time is analysis, not process startup. Splitting these three files does not improve the two-worker schedule: the wall is the longer of the literal file and the other two combined. Those timings are from the reuse change before the lifetime bounds. The later acceptance rerun is recorded under validation and is not a replacement for this table.

For evidence-bearing changes:

Observed, derived, and inferred claims remain distinguishable.
Artifact identity, source provenance, and failed attempts remain preserved.
Unsupported, incomplete, unavailable, or uncertain outcomes remain visible.

Validation performed

node ./node_modules/vitest/vitest.mjs run on the three acceptance files, before the change — 398 passed, 205.0s wall.
The same command after the change — 398 passed, 116.6s wall.
node ./node_modules/vitest/vitest.mjs run tests/boundary/cli/sharedJavaScriptCliSession.test.ts tests/boundary/cli/cliCompileCache.test.ts tests/boundary/cli/sharedJavaScriptMcpSession.test.ts — 20 passed.
npx tsc --noEmit — passed.
npx oxlint on the touched TypeScript and worker files — passed.
Rebased onto main at 335c042d1 (head 4f64c93f4). npx oxfmt --check . passed on 4606 files. tests/composition/reverseEngineeringAdmission.test.ts matches current main, so no extra format commit was required.
node ./node_modules/vitest/vitest.mjs run tests/boundary/cli/doctorCancellation.test.ts tests/boundary/cli/sharedJavaScriptCliSession.test.ts — 14 passed, including Kuna SIGINT and SIGTERM. Current main already parses Kuna version probes as pid-only, so the opaque rea-kuna-version-<uuid> token is not checked as a UUID and #5677 was not cherry-picked.
node ./node_modules/vitest/vitest.mjs run tests/boundary/cli/adbWindowFocus.test.ts — 4 passed. The earlier Vitest worker SIGABRT followed the doctor failures and did not recur on this run.
After 12e3bdb43: npx oxfmt on the touched files, npx oxlint on those files, and npx tsc --noEmit — passed. sharedJavaScriptCliSession.test.ts — 13 passed.
The three acceptance files after the recycle bound — 260 passed, 69.8s wall. The suite is smaller than the earlier 398-test run, so this is a pass-and-time check, not a new before/after table.
Six repeated analyses of a tiny Electron fixture in one worker: resident set 343 MiB on the first command and 416 MiB on the sixth, about 15 MiB of growth per repeat.
After 9a758be2a: a malformed session frame retires the worker before the next queued command. sharedJavaScriptCliSession.test.ts — 14 passed. npx tsc --noEmit passed.

Compatibility, safety, and release

Breaking changes or migration steps: none.
Real Hopper/Ghidra, browser, or OS coverage: not applicable. No provider behavior changed.
Package or release metadata impact: none.
Security, privacy, process, or containment review: the worker is spawned with the same minimal child environment as createTestCli (PATH, compile cache, caller overlay, and the Windows temporary variables). It does not inherit the Vitest process environment. The shared process still stays resident between commands, and it now restarts at 32 commands or a reported 1 GiB resident set.

Author readiness

Human owner: @morluto

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

This PR addresses a concrete problem or an agreed enhancement.
Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff.
The PR has one focused outcome and the title follows type(scope): outcome.
Related issue is linked, or the reason for not linking one is stated above.
Tests cover changed observable behavior and meaningful failure paths.
Owning docs, contracts, and generated metadata are updated where needed.
User-visible CLI/MCP changes include representative output.
I checked the final diff for secrets, unrelated cleanup, and unsupported claims.

_{🔄 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/5676 **Author:** [@morluto](https://github.com/morluto) **Created:** 10/20/2026 **Status:** ✅ Merged **Merged:** 10/21/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `cursor/reuse-js-cli-session-3c30` --- ### 📝 Commits (3) - [`4f64c93`](https://github.com/morluto/rea/commit/4f64c93f4edf7b361afdc918ad0fe4f8f7626457) perf(test): reuse one compiled CLI process per JavaScript application file - [`12e3bdb`](https://github.com/morluto/rea/commit/12e3bdb4364450dd3f3ab46c9c784ffdab167265) fix(test): bound shared CLI worker lifetime - [`9a758be`](https://github.com/morluto/rea/commit/9a758be2ae4b86018a23e3d98c789144785d4d69) fix(test): retire a shared CLI worker after a transport failure ### 📊 Changes **8 files changed** (+1464 additions, -18 deletions) <details> <summary>View changed files</summary> 📝 `docs/testing.md` (+9 -1) ➕ `tests/boundary/cli/sharedJavaScriptCliSession.test.ts` (+577 -0) ➕ `tests/support/cli/cliCommandSession.ts` (+448 -0) ➕ `tests/support/cli/cliCommandSessionFrames.d.mts` (+13 -0) ➕ `tests/support/cli/cliCommandSessionFrames.mjs` (+77 -0) ➕ `tests/support/cli/cliCommandSessionWorker.mjs` (+263 -0) 📝 `tests/support/cli/cliFixture.ts` (+70 -15) 📝 `tests/support/cli/javascriptApplication.ts` (+7 -2) </details> ### 📄 Description  ## Summary After stdio MCP reuse, the slow JavaScript application acceptance files were dominated by fresh CLI processes at about 1.4s each. `analyzeJavaScriptDirectory` now keeps one compiled CLI process per `cliTest` file for the same child environment. Later CLI cases reuse that process. `cli.run` still starts a fresh process. No product issue is linked. This follows the measured gap left by #5658. ## Problem and expected behavior On this host, without coverage, the three slowest files were: - `javascriptLiteralPercentPaths.test.ts`: 191.9s (96 CLI cases, average 1440ms) - `javascriptLoaderReviewBoundaries.test.ts`: 120.5s (62 CLI cases, average 1400ms) - `javascriptIndirectLoaderOwners.test.ts`: 81.4s (41 CLI cases, average 1450ms) Together they took 205.0s wall with Vitest's two workers. MCP cases in the same files already averaged about 540ms. A warm compile cache was already shared. The remaining CLI time was Node and CLI startup around a roughly 0.3s analysis. Those CLI cases should keep every assertion and the same child environment as a fresh process, without paying startup on every case. ## Change and scope `cliTest` owns one file-scoped CLI worker. It loads the compiled runtime once and serves length-prefixed commands. Each command gets the same one-shot argument, environment, and output path as `scripts/rea.mjs`, including a fresh `createCli()`. A different environment restarts the worker. The worker also restarts after 32 commands, when it reports a resident set of at least 1 GiB, and after a fatal command failure or a broken session frame. That failure drops the process before the command settles and waits for it to exit before the next command starts. Incomplete session frames stay in separate chunks until one frame is complete, then that frame is joined once. Stdin and explicit deadlines stay on `cli.run`. File teardown stops the worker, including startup still in flight, and does not open another after close. The worker is test-only. Product CLI and MCP behavior is unchanged. Follow the [contribution scope](https://github.com/morluto/rea/blob/main/CONTRIBUTING.md#contribution-scope). Unresolved scope, authority, side effects, or required verification block integration. Link the required maintainer scope decision for new providers or capabilities involving target execution, injection, mutation, or protection recovery; ordinary fixes within existing boundaries may state "not applicable". Draft PRs may leave the scope decision pending and the readiness and merge checklists unchecked. Maintainers can help resolve open questions. ## Contract and boundary impact - Semantic owner and earliest changed stage: test harness. The shared process starts at the first `analyzeJavaScriptDirectory` CLI call in a `cliTest` file. - Contribution scope fit and linked maintainer decision: not applicable. Test layout and the existing compiled CLI entry only. - CLI and MCP/tool-catalog contract: none. `cli.run` and MCP sessions are unchanged. - Provider, bridge, target-format, or platform compatibility: none. - Evidence, artifact, provenance, or reconstruction contract: none. A regression compares `normalized_result` with a fresh process. - Target selection, authority, upstream behavior, and cleanup details (if applicable): the file fixture owns the worker and stops it on teardown. - Generated metadata (`docs/public/product-catalog.json`), package, or installation impact (CI generates and validates ignored catalogs and `skills/`; edit authored instructions in `.agents/skills/reverse-engineer-anything/` and never commit build outputs): none. Expected effects (check known effects; use documentation-only on its own). Unchecked items do not establish absence; leave uncertainties visible. - [ ] Read existing files or captured data. - [x] Launch or attach to a process. - [ ] Inject instrumentation or run scripts/code. - [ ] Change process memory, registers, files, or configuration. - [ ] Contact network services. - [ ] Recover protected code or work around protections. - [ ] Documentation or usability only; no target interaction. - [ ] Not sure yet; investigation or maintainer guidance needed. The launched process is the test worker. It does not change product launch behavior. ## Evidence and regression coverage - Tests added or updated: `tests/boundary/cli/sharedJavaScriptCliSession.test.ts` covers frame parsing, including a frame delivered one byte at a time, close during startup and in-flight work, output isolation, environment restart, exit recovery, fatal retirement before the next spawn, transport-failure retirement before the next spawn, restart after a command limit and after a reported resident set, byte-level `--version` and invalid `REA_LOG_LEVEL` parity with a fresh process, and equal `normalized_result` across two application directories. `analyzeJavaScriptDirectory` is included in that parity check. - Base reproduction or other evidence: same three acceptance files, Node 24.18.0, 4 cores, no coverage, one paired run. Process-family CPU is the sum of sampled `/proc` utime+stime (100 Hz). RSS is the sum of VmRSS and can count shared pages more than once. PSS is from `smaps_rollup`; 82 of 1023 before-samples and 65 of 580 after-samples had no PSS. - User-visible CLI/MCP output (if applicable): not applicable. - Remaining proof gaps: sibling `analyzeJavaScriptDirectory` files were not re-timed as a group. One run does not estimate CI variance. Coverage mode was not measured. The worker repeats the `scripts/rea.mjs` one-shot sequence, so a later entry change is guarded by the fresh-process comparison rather than a shared function. The 1 GiB and 32-command caps were checked with fakes plus six repeats of a tiny Electron fixture; process-family PSS for the original three-file run was not measured again. | | Before | After | | --- | ---: | ---: | | Wall | 205.0s | 116.6s | | Process-family CPU | 420.7s | 207.9s | | Peak summed RSS | 2740 MiB | 4120 MiB | | Peak summed PSS | 2332 MiB | 3815 MiB | | Literal file | 191.9s | 105.8s | | Loader file | 120.5s | 68.4s | | Indirect file | 81.4s | 44.5s | | CLI case average | 1400–1450ms | 530–567ms | All 398 tests passed in both runs. CLI case time now matches the MCP cases in the same files (about 530–570ms). The remaining time is analysis, not process startup. Splitting these three files does not improve the two-worker schedule: the wall is the longer of the literal file and the other two combined. Those timings are from the reuse change before the lifetime bounds. The later acceptance rerun is recorded under validation and is not a replacement for this table. For evidence-bearing changes: - [ ] Observed, derived, and inferred claims remain distinguishable. - [ ] Artifact identity, source provenance, and failed attempts remain preserved. - [ ] Unsupported, incomplete, unavailable, or uncertain outcomes remain visible. ## Validation performed - `node ./node_modules/vitest/vitest.mjs run` on the three acceptance files, before the change — 398 passed, 205.0s wall. - The same command after the change — 398 passed, 116.6s wall. - `node ./node_modules/vitest/vitest.mjs run tests/boundary/cli/sharedJavaScriptCliSession.test.ts tests/boundary/cli/cliCompileCache.test.ts tests/boundary/cli/sharedJavaScriptMcpSession.test.ts` — 20 passed. - `npx tsc --noEmit` — passed. - `npx oxlint` on the touched TypeScript and worker files — passed. - Rebased onto `main` at `335c042d1` (head `4f64c93f4`). `npx oxfmt --check .` passed on 4606 files. `tests/composition/reverseEngineeringAdmission.test.ts` matches current `main`, so no extra format commit was required. - `node ./node_modules/vitest/vitest.mjs run tests/boundary/cli/doctorCancellation.test.ts tests/boundary/cli/sharedJavaScriptCliSession.test.ts` — 14 passed, including Kuna SIGINT and SIGTERM. Current `main` already parses Kuna version probes as pid-only, so the opaque `rea-kuna-version-<uuid>` token is not checked as a UUID and #5677 was not cherry-picked. - `node ./node_modules/vitest/vitest.mjs run tests/boundary/cli/adbWindowFocus.test.ts` — 4 passed. The earlier Vitest worker SIGABRT followed the doctor failures and did not recur on this run. - After `12e3bdb43`: `npx oxfmt` on the touched files, `npx oxlint` on those files, and `npx tsc --noEmit` — passed. `sharedJavaScriptCliSession.test.ts` — 13 passed. - The three acceptance files after the recycle bound — 260 passed, 69.8s wall. The suite is smaller than the earlier 398-test run, so this is a pass-and-time check, not a new before/after table. - Six repeated analyses of a tiny Electron fixture in one worker: resident set 343 MiB on the first command and 416 MiB on the sixth, about 15 MiB of growth per repeat. - After `9a758be2a`: a malformed session frame retires the worker before the next queued command. `sharedJavaScriptCliSession.test.ts` — 14 passed. `npx tsc --noEmit` passed. ## Compatibility, safety, and release - Breaking changes or migration steps: none. - Real Hopper/Ghidra, browser, or OS coverage: not applicable. No provider behavior changed. - Package or release metadata impact: none. - Security, privacy, process, or containment review: the worker is spawned with the same minimal child environment as `createTestCli` (`PATH`, compile cache, caller overlay, and the Windows temporary variables). It does not inherit the Vitest process environment. The shared process still stays resident between commands, and it now restarts at 32 commands or a reported 1 GiB resident set. ## Author readiness Human owner: @morluto - [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. ## Before merge - [x] This PR addresses a concrete problem or an agreed enhancement. - [x] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [x] The PR has one focused outcome and the title follows `type(scope): outcome`. - [x] Related issue is linked, or the reason for not linking one is stated above. - [x] Tests cover changed observable behavior and meaningful failure paths. - [x] Owning docs, contracts, and generated metadata are updated where needed. - [x] User-visible CLI/MCP changes include representative output. - [x] I checked the final diff for secrets, unrelated cleanup, and unsupported claims.  <div><a href="https://cursor.com/agents/bc-cb65d626-32f7-5d94-b108-76ca2edf3c30?cursor_ref=pr_footer&cursor_cta=open_in_web"><picture><source media="(prefers-color-scheme: dark)" srcset="https://cursor.com/assets/images/open-in-web-dark.png"><source media="(prefers-color-scheme: light)" srcset="https://cursor.com/assets/images/open-in-web-light.png"><img alt="Open in Web" width="114" height="28" src="https://cursor.com/assets/images/open-in-web-dark.png"></picture></a> <a href="https://cursor.com/background-agent?bcId=bc-cb65d626-32f7-5d94-b108-76ca2edf3c30&cursor_ref=pr_footer&cursor_cta=open_in_cursor"><picture><source media="(prefers-color-scheme: dark)" srcset="https://cursor.com/assets/images/open-in-cursor-dark.png"><source media="(prefers-color-scheme: light)" srcset="https://cursor.com/assets/images/open-in-cursor-light.png"><img alt="Open in Cursor" width="131" height="28" src="https://cursor.com/assets/images/open-in-cursor-dark.png"></picture></a> </div> --- <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:51 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-11-03 13:38:55 +00:00

[PR #7032] [MERGED] fix(setup): materialize the pinned package-runner cache before the first client start #7588

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

No description provided.

Rows
Columns