[PR #5679] [MERGED] perf(test): reuse one compiled CLI for export-binding parity #7033

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/5679
Author: @morluto
Created: 10/20/2026
Status: ✅ Merged
Merged: 10/21/2026
Merged by: @morluto

Base: main ← Head: cursor/warm-export-binding-cli-bf52

📝 Commits (6)

e034abc perf(test): reuse one compiled CLI for export-binding parity
733f487 style(test): format the warm CLI session
465158c fix(test): report warm CLI shutdown, deadlines, and exit status
5b62723 fix(test): close the warm CLI worker when a status check fails
5d8c094 fix(test): retain a timed-out compiled CLI worker until it exits
666cc06 fix(test): reject a compiled CLI launch that close interrupts

📊 Changes

5 files changed (+867 additions, -29 deletions)

View changed files

📝 docs/testing.md (+7 -0)
📝 tests/acceptance/applications/exportedFunctionBindingsCli.test.ts (+11 -29)
➕ tests/boundary/cli/compiledCliSession.test.ts (+266 -0)
➕ tests/support/cli/compiledCliSession.ts (+444 -0)
➕ tests/support/cli/compiledCliWorker.mjs (+139 -0)

📄 Description

Summary

exportedFunctionBindingsCli.test.ts was the long file on the fast built shard. Its 26 slow cases each started the compiled CLI three times. Those cases now share one warm CLI process and still compare that process's JSON with the stdio MCP session. Focused runs of the same 92 tests dropped from 145.2s to 51.1s.

Human owner: @morluto.

Problem and expected behavior

On the post-#5652 CI logs, this file was about 240s and set shard 3/8 (run 39485164086, about 11.3 minutes). MCP reuse barely moved it. Locally, the same file was 145.2s: 26 cases that call expectCliAnalysisParity plus compareThroughCli took about 4.0–4.4s each, while MCP-only cases took about 0.35–0.5s.

A warm scripts/rea.mjs --version was 0.7–0.9s and a one-file analysis spawn was about 1.1s. The same analysis inside an already loaded CLI was about 0.3s. The slow tail is process startup, not the fixture analysis. Splitting the file would not remove those spawns.

Change and scope

The file keeps one compiled CLI child for the file. The child imports dist/cli-runtime once and serves the same --json commands as scripts/rea.mjs. It reuses one createCli instance. A new instance per command retained command registrations and grew about 8–10 MiB of RSS per analysis (291 MiB to 549 MiB over 30 identical calls). One instance stayed at 289 MiB to 308 MiB over those 30 calls.

A command that does not respond is rejected. That process stays owned until it exits, and another process does not start while it remains. A process that survives SIGKILL stays owned so a later close can retry the stop. Closing during the ready handshake rejects the launch instead of leaving it until the startup timer. Closing the session still stops a child that ignores stdin, and that close reports the forced shutdown. logCliCommand failures that only set process.exitCode stay status 1 for the next command.

Assertions are unchanged. CLI normalized_result is still compared with the MCP result. analyzeJavaScriptDirectory CLI cases share one compiled process per file. This file keeps its own warm process. docs/testing.md describes both.

Not included: resharding, other loader CLI files, and changes to production analysis.

Contribution scope: not applicable. This is test harness work inside the existing CLI/MCP parity checks.

Contract and boundary impact

Semantic owner and earliest changed stage: the acceptance test owns the warm CLI child. Command results still come from the compiled CLI runtime.
Contribution scope fit and linked maintainer decision: not applicable.
CLI and MCP/tool-catalog contract: none. Production commands are unchanged.
Provider, bridge, target-format, or platform compatibility: none.
Evidence, artifact, provenance, or reconstruction contract: none. The tests still require CLI/MCP normalized_result equality.
Target selection, authority, upstream behavior, and cleanup details (if applicable): the child is closed in afterAll. A timed-out child stays owned until exit. A close during startup rejects the launch before the child is detached. If it does not exit after stdin closes, cleanup sends SIGTERM and then SIGKILL and reports that graceful shutdown failed. A child that survives SIGKILL stays owned for a later close.
Generated metadata (docs/public/product-catalog.json), package, or installation impact: none.

Expected effects: test-only. No product target interaction.

Evidence and regression coverage

Tests added or updated: tests/acceptance/applications/exportedFunctionBindingsCli.test.ts uses the warm session. No assertions were removed. tests/support/cli/compiledCliSession.ts and compiledCliWorker.mjs own the child. tests/boundary/cli/compiledCliSession.test.ts covers a never-replying worker, a delayed exit before the next command, a SIGKILL that does not stop the child, a close that wins the ready handshake, a forced stop, and a failed command followed by a successful one. That real-worker case registers close() with onTestFinished so a failed assertion still stops the child.
Base reproduction: focused run at a7d13893 (this test file is unchanged from there through c38f96fa). Host Linux, Node v22.23.3, 4 CPUs. npm run test:focused -- tests/acceptance/applications/exportedFunctionBindingsCli.test.ts.
- Before: 92 passed, Vitest file time 145.16s (duration 145.84s). Process-family wall 146.9s, observed CPU 190.17s, peak summed RSS 1,230,520 KiB. PSS was not recorded.
- After, on c38f96fa plus this change: 92 passed, Vitest file time 51.12s (duration 51.71s). npm wall 64.0s includes an 11.3s compile cache miss. Observed CPU 59.83s, peak summed RSS 1,429,384 KiB, peak summed PSS 1,061,289 KiB.
- RSS is a 200ms sum and double-counts shared pages. PSS is summed every 1s. CPU is utime+stime of processes seen at least once.
User-visible CLI/MCP output (if applicable): none.
Remaining proof gaps: no new CI shard timing. Family peak RSS is about 190 MiB above the fresh-process baseline because the CLI stays resident. Two different fixtures matched a fresh process's normalized_result before the file run; the file run is the parity oracle.

Validation performed

npm run test:focused -- tests/acceptance/applications/exportedFunctionBindingsCli.test.ts at a7d13893 — 92 passed, file time 145.16s.
Same command on this branch — 92 passed, file time 51.12s. An earlier run of the same harness was 52.37s.
npx tsc --noEmit — passed.
npx oxfmt --check on the touched session, worker, test, and docs/testing.md — passed.
Worker probe: 30 analyses of one fixture. New CLI each call grew 291 MiB to 549 MiB RSS. One reused CLI grew 289 MiB to 308 MiB, and stdout length stayed 27,445 bytes.
Head 666cc0663 is rebased onto 044f18022 (#5677). npm run test:focused -- tests/boundary/cli/compiledCliSession.test.ts — 7 passed.

Compatibility, safety, and release

Breaking changes or migration steps: none.
Real Hopper/Ghidra, browser, or OS coverage: not applicable. These cases use local JavaScript fixtures through the compiled CLI and stdio MCP.
Package or release metadata impact: none.
Security, privacy, process, or containment review: the warm child is test-owned and closed with the file. It does not change production process authority.

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.

_{🔄 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/5679 **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/warm-export-binding-cli-bf52` --- ### 📝 Commits (6) - [`e034abc`](https://github.com/morluto/rea/commit/e034abc47dd56300b9589017a865685c1e32f07b) perf(test): reuse one compiled CLI for export-binding parity - [`733f487`](https://github.com/morluto/rea/commit/733f48781726df0d94d914d5fadd32ec0d8ebb53) style(test): format the warm CLI session - [`465158c`](https://github.com/morluto/rea/commit/465158c119c396726e144ba38ebc9647de7c6ad2) fix(test): report warm CLI shutdown, deadlines, and exit status - [`5b62723`](https://github.com/morluto/rea/commit/5b627238443fe53ad0852fdeb3ad2c933b237beb) fix(test): close the warm CLI worker when a status check fails - [`5d8c094`](https://github.com/morluto/rea/commit/5d8c094b22adf3c36bab9ee1770c9117a09ab20c) fix(test): retain a timed-out compiled CLI worker until it exits - [`666cc06`](https://github.com/morluto/rea/commit/666cc06634b6970a8dd0399def919a440e37f482) fix(test): reject a compiled CLI launch that close interrupts ### 📊 Changes **5 files changed** (+867 additions, -29 deletions) <details> <summary>View changed files</summary> 📝 `docs/testing.md` (+7 -0) 📝 `tests/acceptance/applications/exportedFunctionBindingsCli.test.ts` (+11 -29) ➕ `tests/boundary/cli/compiledCliSession.test.ts` (+266 -0) ➕ `tests/support/cli/compiledCliSession.ts` (+444 -0) ➕ `tests/support/cli/compiledCliWorker.mjs` (+139 -0) </details> ### 📄 Description  ## Summary `exportedFunctionBindingsCli.test.ts` was the long file on the fast built shard. Its 26 slow cases each started the compiled CLI three times. Those cases now share one warm CLI process and still compare that process's JSON with the stdio MCP session. Focused runs of the same 92 tests dropped from 145.2s to 51.1s. Human owner: @morluto. ## Problem and expected behavior On the post-#5652 CI logs, this file was about 240s and set shard 3/8 (run 39485164086, about 11.3 minutes). MCP reuse barely moved it. Locally, the same file was 145.2s: 26 cases that call `expectCliAnalysisParity` plus `compareThroughCli` took about 4.0–4.4s each, while MCP-only cases took about 0.35–0.5s. A warm `scripts/rea.mjs --version` was 0.7–0.9s and a one-file analysis spawn was about 1.1s. The same analysis inside an already loaded CLI was about 0.3s. The slow tail is process startup, not the fixture analysis. Splitting the file would not remove those spawns. ## Change and scope The file keeps one compiled CLI child for the file. The child imports `dist/cli-runtime` once and serves the same `--json` commands as `scripts/rea.mjs`. It reuses one `createCli` instance. A new instance per command retained command registrations and grew about 8–10 MiB of RSS per analysis (291 MiB to 549 MiB over 30 identical calls). One instance stayed at 289 MiB to 308 MiB over those 30 calls. A command that does not respond is rejected. That process stays owned until it exits, and another process does not start while it remains. A process that survives SIGKILL stays owned so a later close can retry the stop. Closing during the ready handshake rejects the launch instead of leaving it until the startup timer. Closing the session still stops a child that ignores stdin, and that close reports the forced shutdown. `logCliCommand` failures that only set `process.exitCode` stay status 1 for the next command. Assertions are unchanged. CLI `normalized_result` is still compared with the MCP result. `analyzeJavaScriptDirectory` CLI cases share one compiled process per file. This file keeps its own warm process. `docs/testing.md` describes both. Not included: resharding, other loader CLI files, and changes to production analysis. Contribution scope: not applicable. This is test harness work inside the existing CLI/MCP parity checks. ## Contract and boundary impact - Semantic owner and earliest changed stage: the acceptance test owns the warm CLI child. Command results still come from the compiled CLI runtime. - Contribution scope fit and linked maintainer decision: not applicable. - CLI and MCP/tool-catalog contract: none. Production commands are unchanged. - Provider, bridge, target-format, or platform compatibility: none. - Evidence, artifact, provenance, or reconstruction contract: none. The tests still require CLI/MCP `normalized_result` equality. - Target selection, authority, upstream behavior, and cleanup details (if applicable): the child is closed in `afterAll`. A timed-out child stays owned until exit. A close during startup rejects the launch before the child is detached. If it does not exit after stdin closes, cleanup sends SIGTERM and then SIGKILL and reports that graceful shutdown failed. A child that survives SIGKILL stays owned for a later close. - Generated metadata (`docs/public/product-catalog.json`), package, or installation impact: none. Expected effects: test-only. No product target interaction. ## Evidence and regression coverage - Tests added or updated: `tests/acceptance/applications/exportedFunctionBindingsCli.test.ts` uses the warm session. No assertions were removed. `tests/support/cli/compiledCliSession.ts` and `compiledCliWorker.mjs` own the child. `tests/boundary/cli/compiledCliSession.test.ts` covers a never-replying worker, a delayed exit before the next command, a SIGKILL that does not stop the child, a close that wins the ready handshake, a forced stop, and a failed command followed by a successful one. That real-worker case registers `close()` with `onTestFinished` so a failed assertion still stops the child. - Base reproduction: focused run at `a7d13893` (this test file is unchanged from there through `c38f96fa`). Host Linux, Node v22.23.3, 4 CPUs. `npm run test:focused -- tests/acceptance/applications/exportedFunctionBindingsCli.test.ts`. - Before: 92 passed, Vitest file time 145.16s (duration 145.84s). Process-family wall 146.9s, observed CPU 190.17s, peak summed RSS 1,230,520 KiB. PSS was not recorded. - After, on `c38f96fa` plus this change: 92 passed, Vitest file time 51.12s (duration 51.71s). npm wall 64.0s includes an 11.3s compile cache miss. Observed CPU 59.83s, peak summed RSS 1,429,384 KiB, peak summed PSS 1,061,289 KiB. - RSS is a 200ms sum and double-counts shared pages. PSS is summed every 1s. CPU is utime+stime of processes seen at least once. - User-visible CLI/MCP output (if applicable): none. - Remaining proof gaps: no new CI shard timing. Family peak RSS is about 190 MiB above the fresh-process baseline because the CLI stays resident. Two different fixtures matched a fresh process's `normalized_result` before the file run; the file run is the parity oracle. ## Validation performed - `npm run test:focused -- tests/acceptance/applications/exportedFunctionBindingsCli.test.ts` at `a7d13893` — 92 passed, file time 145.16s. - Same command on this branch — 92 passed, file time 51.12s. An earlier run of the same harness was 52.37s. - `npx tsc --noEmit` — passed. - `npx oxfmt --check` on the touched session, worker, test, and `docs/testing.md` — passed. - Worker probe: 30 analyses of one fixture. New CLI each call grew 291 MiB to 549 MiB RSS. One reused CLI grew 289 MiB to 308 MiB, and stdout length stayed 27,445 bytes. - Head `666cc0663` is rebased onto `044f18022` (#5677). `npm run test:focused -- tests/boundary/cli/compiledCliSession.test.ts` — 7 passed. ## Compatibility, safety, and release - Breaking changes or migration steps: none. - Real Hopper/Ghidra, browser, or OS coverage: not applicable. These cases use local JavaScript fixtures through the compiled CLI and stdio MCP. - Package or release metadata impact: none. - Security, privacy, process, or containment review: the warm child is test-owned and closed with the file. It does not change production process authority. ## 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.  <div><a href="https://cursor.com/agents/bc-6b52df15-1023-5b31-9a99-c5b257e5bf52?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-6b52df15-1023-5b31-9a99-c5b257e5bf52&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