morluto/rea

Fork 0

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

[PR #5819] [MERGED] perf(javascript): reuse the source-analysis worker within a process #7083

New issue

Closed

opened 2026-11-01 21:45:15 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-11-01 21:45:15 +00:00

Owner

📋 Pull Request Information

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

Base: main ← Head: cursor/reuse-js-analysis-worker-dc95

📝 Commits (10+)

a02c4aa perf(javascript): reuse the source-analysis worker within a process
1a98f89 fix(cli): close the JavaScript worker when dispatch throws
bc41270 fix(artifacts): report close-hook failures with retained cleanup
55b358d fix(javascript): stop a replaced analysis worker before the next one starts
9ab49e6 style: format worker-reuse close and launch helpers
407069e merge: bring origin/main into the JavaScript worker reuse branch
40cd0f4 merge: bring origin/main into the JavaScript worker reuse branch
1c55374 fix(cli): report a failed JavaScript worker shutdown before exit
aee3828 fix(javascript): keep a worker parked during an overlapping limit change
69e1434 fix(artifacts): run later close hooks when an earlier hook fails

📊 Changes

15 files changed (+925 additions, -77 deletions)

View changed files

📝 scripts/rea.mjs (+94 -39)
📝 src/application/javascript/JavaScriptArtifactReconstruction.ts (+2 -1)
📝 src/artifacts/ArtifactResourceScope.test.ts (+83 -0)
📝 src/artifacts/ArtifactResourceScope.ts (+95 -1)
📝 src/artifacts/javascript/JavaScriptSourceAnalysis.ts (+145 -2)
📝 src/cli.ts (+2 -0)
📝 src/cli/javascriptApplicationAnalysis.ts (+40 -3)
📝 src/javascript/analysis/JavaScriptAnalysisWorker.ts (+45 -3)
➕ tests/boundary/cli/cliJavaScriptResourceClose.test.ts (+66 -0)
📝 tests/boundary/cli/dispatcher.test.ts (+65 -22)
📝 tests/boundary/filesystem/javascript-applications/javascriptApplicationService.test.ts (+1 -1)
📝 tests/boundary/javascript/javascriptSourceAnalysisWorker.test.ts (+274 -0)
📝 tests/boundary/javascriptSemanticTraceEvidence.test.ts (+6 -2)
📝 tests/support/cli/cliCommandSessionWorker.mjs (+5 -2)
📝 tests/support/cli/compiledCliWorker.mjs (+2 -1)

📄 Description

Summary

Sequential analyze-javascript-application calls each started a new Node analysis worker. That process startup, not fixture I/O or the already-shared CLI session, was the large cost in the slowest JavaScript acceptance files. A healthy worker now stays parked on the artifact resource scope (and on one CLI-process scope) until that owner closes.

Problem and expected behavior

CI fast timings from run 39553734979 (d672cc239, test-results / test-timings.json; two commits before the previous main) rank the suite at 6515s across 1898 files. Acceptance, boundary, MCP, and CLI are most of that. The longest files:

Seconds	File
116.7	`tests/acceptance/applications/javascriptLoaderReviewBoundaries.test.ts`
116.7	`tests/acceptance/applications/exportedFunctionBindingsCli.test.ts`
81.3	`tests/boundary/cli/clientProfiles.test.ts`
61.6	`tests/acceptance/setup/piClientSetup.test.ts`
57.5	`tests/acceptance/applications/javascriptFactoryCompletion.test.ts`
54.4	`tests/boundary/mcp/analysisViewMcp.test.ts`
54.2	`tests/acceptance/setup/registrationRuntimeEnvironmentCli.test.ts`
53.1	`tests/acceptance/tools/qlogCapture.test.ts`
51.8	`tests/acceptance/applications/javascriptIndirectLoaderOwners.test.ts`
51.5	`tests/acceptance/setup/hermesClientSetup.test.ts`
48.9	`tests/boundary/mcp/callRelationshipAvailability.test.ts`
46.5	`tests/acceptance/process/processFinalization.test.ts`
45.9	`tests/acceptance/applications/mobileDirectoryComponents.test.ts`
45.0	`tests/acceptance/electronFusesCli.test.ts`
44.6	`tests/acceptance/setup/dshClientSetup.test.ts`

A CPU profile of one worker startup (215ms, 185 samples) was compileSourceTextModule 23.2%, GC 8.1%, idle 7.6%. The ready handshake alone was about 164–216ms. Bundling the worker and NODE_COMPILE_CACHE only moved ready to about 126–140ms and 134ms, so those were not shipped. After the process was warm, per-file semantic work was spread across domain/javascript with no single function worth a separate change. Zod parse of the result was 9–28ms.

Warm compiled-CLI commands on the loader “returned module” fixture were 405–607ms, with a 242–294ms gap before the first JavaScript file. The shared CLI session was already warm; the worker child was the gap.

Expected behavior: a later analysis on the same scope and the same heap/timeout reuses that process. A failed, cancelled, or limit-changed analysis does not. Scope close, session stdin end, and one-shot process.exit still stop the process and remove its private root. CLI and MCP results stay the same. A limit change closes the previous worker before the replacement is constructed, and does not replace a worker a concurrent analysis parked during that close. If that close fails, the request fails and the still-live worker stays owned; a replacement is not started. Dispatch cleanup still runs when serve or output rendering throws. A failed shutdown stays owned for a retry and is reported before the one-shot process exits. A closed stdout pipe still exits 141.

Change and scope

createJavaScriptSourceAnalysis keeps one idle worker per ArtifactResourceScope. Reconstruction already passes the caller’s scope, so MCP uses the server scope that createServer closes on shutdown. The CLI keeps one process-lived scope, retries failed owners without closing a healthy worker, and closeCliJavaScriptResources runs from scripts/rea.mjs (including a finally around dispatch), the compiled CLI session, and the command session. In-process javascriptSemanticTraceEvidence closes that scope in afterAll.

The CLI shutdown is one shared promise. The scope stays owned until that close succeeds, so the exit callback and the dispatch finally observe the same attempt. The exit callback writes a cleanup failure and exits non-zero; a closed stdout pipe still exits 141. Different heapMb / timeoutMs close the parked process, then start another. If that close fails, checkout throws before constructing a replacement and puts the still-live worker back in the slot when it is empty. The replacement claims the slot only when it is still empty; otherwise the extra process is closed through its own port. Result-budget admission is refreshed from the current parent heap on reuse. Every close hook runs. A close that fails both retained cleanup and a close hook reports both resources and both causes.

Follow the contribution scope. This stays inside the existing static JavaScript analyzer. It does not add target execution, injection, mutation, or protection recovery. Maintainer scope decision: not applicable.

Contract and boundary impact

Semantic owner and earliest changed stage: the JavaScript source-analysis worker, owned by the artifact resource scope that already owns the analysis. The change is at worker acquisition, before parsing.
Contribution scope fit and linked maintainer decision: ordinary performance fix inside the existing analyzer. Not applicable.
CLI and MCP/tool-catalog contract: none. Same command, arguments, and result schema. The fixture replay kept a 141018-byte stdout. A failed worker shutdown on process exit is now a non-zero status, except a closed stdout pipe, which stays 141. A limit change whose previous worker cannot be stopped fails the analysis instead of returning success with two owned processes.
Provider, bridge, target-format, or platform compatibility: none. The worker is still the owned Node child.
Evidence, artifact, provenance, or reconstruction contract: unchanged. Factory completion still requires CLI and MCP evidence_id parity.
Target selection, authority, upstream behavior, and cleanup details (if applicable): no new authority. The detached worker is still stopped by scope close. Failed cleanup still fails the CLI command via retryCleanup. A close hook still runs when other retained cleanup is failing, and both failures stay visible. Later hooks still run when an earlier hook fails.
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.

Evidence and regression coverage

Tests added or updated: worker reuse across one scope, a different heap, and scope close, including that the previous process has exited before the next spawn, that a worker parked during a limit-change close is not replaced, and that a failed close does not start a replacement (javascriptSourceAnalysisWorker.test.ts); close hook still runs when retained cleanup fails, both failures are reported, and a later hook still runs when an earlier hook fails (ArtifactResourceScope.test.ts); CLI shutdown retries after a failed close and shares one in-flight close (cliJavaScriptResourceClose.test.ts); dispatcher reports a failed shutdown as exit 1 and keeps closed-pipe status 141 (dispatcher.test.ts); in-process CLI closes the parked worker (javascriptSemanticTraceEvidence.test.ts).
Base reproduction or other evidence: same-VM npm run test:focused on main’s compile, then on this branch. Loader review 60.28s → 30.77s (124 passed both). Factory completion 34.88s → 21.57s; the 40-scenario test 24482ms → 11595ms. The before runs were second, so disk cache favored the slower baseline.
User-visible CLI/MCP output (if applicable): unchanged on the profiled fixture (stdout 141018 bytes, exit 0). A failed worker shutdown during one-shot exit is now written to stderr and is a non-zero status. A limit change that cannot stop the previous worker fails the analysis.
Remaining proof gaps: exportedFunctionBindingsCli.test.ts and javascriptIndirectLoaderOwners.test.ts use the same command and were not timed again. Client-profile, setup, qlog, and process-finalization files are a different cost and were not changed. A long MCP session’s child heap was not load-tested; the worker already collects when heapUsed exceeds half its limit. Full CI shard wall was not re-measured. peResources acceptance timing out at 30s is the same failure on current main and is not changed here.

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

npm run test:focused -- tests/acceptance/applications/javascriptLoaderReviewBoundaries.test.ts — before 60.28s / 124 passed; after 30.77s / 124 passed.
npm run test:focused -- tests/acceptance/applications/javascriptFactoryCompletion.test.ts — before 34.88s (24482ms and 7974ms); after 21.57s (11595ms and 7166ms).
npm run test:focused -- tests/boundary/javascript/javascriptSourceAnalysisWorker.test.ts tests/boundary/cli/cliJavaScriptResourceClose.test.ts tests/boundary/cli/dispatcher.test.ts tests/acceptance/applications/peResources.test.ts — 21 passed. The new case does not start a replacement when closing the previous worker fails passed in 544ms. peResources passed locally in 18.9s.
npm run format:check — passed (4676 files).
npm run check:fast — typecheck and lint passed.
Compiled CLI session on the loader fixture — warm commands 405–607ms before; 546ms then 172–189ms after; session exit 0.
CI shard 7/8 on this head 69e1434b (run 39585068374, job 119172814208) and on main c324d31b (run 39584670117, job 119171892915) both fail only tests/acceptance/applications/peResources.test.ts with Test timed out in 30000ms. Boundary tests/boundary/filesystem/peResources.test.ts passed on both shards (562ms and 463ms). This PR does not change PE inspection.
Real Hopper/Ghidra: not applicable. This path does not call them.

Compatibility, safety, and release

Breaking changes or migration steps: none.
Real Hopper/Ghidra, browser, or OS coverage: not used. Static JavaScript analysis only, on Linux in this VM.
Package or release metadata impact: none.
Security, privacy, process, or containment review: the worker stays detached and owned. It is not reused after a failed, cancelled, or dirty protocol state. Close still removes the private rea-provider- root. One-shot CLI process.exit reports a failed worker shutdown and exits non-zero, except a closed stdout pipe, which stays 141. A thrown dispatch still reaches that cleanup. A replaced worker is closed before the next process starts. A failed close does not start that next process. A worker parked during that close stays owned.

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

Profile evidence, same VM:

Worker startup, 215ms / 185 samples: compileSourceTextModule 23.2%, GC 8.1%, idle 7.6%. Ready handshake about 164–216ms. Per-file work after ready had no single dominant frame.
Warm CLI phase gap before the first file: about 242–294ms before, gone on the second command after reuse (progress at 1ms, a/package.json at 45ms).
javascriptLoaderReviewBoundaries.test.ts: 60.28s → 30.77s. Steady cases fell under Vitest’s 300ms slow-test print; they were about 420–460ms before.
javascriptFactoryCompletion.test.ts: 34.88s → 21.57s (hot test 24482ms → 11595ms).

Review follow-up on 2a94716e: a limit change whose previous worker cannot be stopped fails the request before a replacement starts, and that worker stays owned. Shutdown remains one shared close. A limit change claims the slot only when it is still empty. Later close hooks still run when an earlier hook fails.

peResources acceptance timeout is the same single failure on main c324d31b shard 7/8. It is not fixed in this PR.

This branch includes origin/main through 5eebe2fd, which treats omitted exactOptional inputs as optional. No product conflict edits were required.

Not changed: shard count, cancel-in-progress, workflow concurrency, the shared compiled CLI session from #5676 / #5784, and PE resource tests.

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.

No linked issue: this follows measured CI file time after #5799, and the timings above are the report. No user-visible CLI/MCP output change on the successful analysis path, so there is no new sample output. A failed worker shutdown on one-shot exit, and a limit change that cannot stop the previous worker, are error paths.

_{🔄 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/5819 **Author:** [@morluto](https://github.com/morluto) **Created:** 10/21/2026 **Status:** ✅ Merged **Merged:** 10/21/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `cursor/reuse-js-analysis-worker-dc95` --- ### 📝 Commits (10+) - [`a02c4aa`](https://github.com/morluto/rea/commit/a02c4aa501d9a46c2add27e5cb61241f91ecdca6) perf(javascript): reuse the source-analysis worker within a process - [`1a98f89`](https://github.com/morluto/rea/commit/1a98f89bb2ea75e7f1c62f014d88968228a9a26d) fix(cli): close the JavaScript worker when dispatch throws - [`bc41270`](https://github.com/morluto/rea/commit/bc4127013d37e4ac82a69250b3019a84b965d7d2) fix(artifacts): report close-hook failures with retained cleanup - [`55b358d`](https://github.com/morluto/rea/commit/55b358de2005167de55f3fadb6957ac55962da55) fix(javascript): stop a replaced analysis worker before the next one starts - [`9ab49e6`](https://github.com/morluto/rea/commit/9ab49e613f0c8ac986464b715ab5015e1d580cdd) style: format worker-reuse close and launch helpers - [`407069e`](https://github.com/morluto/rea/commit/407069edd24f7a2e8377467d6cfaa236834ab5d6) merge: bring origin/main into the JavaScript worker reuse branch - [`40cd0f4`](https://github.com/morluto/rea/commit/40cd0f42c7c6d0c6016c65b1258515dbbbe45fea) merge: bring origin/main into the JavaScript worker reuse branch - [`1c55374`](https://github.com/morluto/rea/commit/1c553748a7ab85d6c0175cb1f1b2669d72ac2ffc) fix(cli): report a failed JavaScript worker shutdown before exit - [`aee3828`](https://github.com/morluto/rea/commit/aee382857fcc82c0de239bd30a20a01974b6c6df) fix(javascript): keep a worker parked during an overlapping limit change - [`69e1434`](https://github.com/morluto/rea/commit/69e1434b57b14f67abaef936643b3b731051f251) fix(artifacts): run later close hooks when an earlier hook fails ### 📊 Changes **15 files changed** (+925 additions, -77 deletions) <details> <summary>View changed files</summary> 📝 `scripts/rea.mjs` (+94 -39) 📝 `src/application/javascript/JavaScriptArtifactReconstruction.ts` (+2 -1) 📝 `src/artifacts/ArtifactResourceScope.test.ts` (+83 -0) 📝 `src/artifacts/ArtifactResourceScope.ts` (+95 -1) 📝 `src/artifacts/javascript/JavaScriptSourceAnalysis.ts` (+145 -2) 📝 `src/cli.ts` (+2 -0) 📝 `src/cli/javascriptApplicationAnalysis.ts` (+40 -3) 📝 `src/javascript/analysis/JavaScriptAnalysisWorker.ts` (+45 -3) ➕ `tests/boundary/cli/cliJavaScriptResourceClose.test.ts` (+66 -0) 📝 `tests/boundary/cli/dispatcher.test.ts` (+65 -22) 📝 `tests/boundary/filesystem/javascript-applications/javascriptApplicationService.test.ts` (+1 -1) 📝 `tests/boundary/javascript/javascriptSourceAnalysisWorker.test.ts` (+274 -0) 📝 `tests/boundary/javascriptSemanticTraceEvidence.test.ts` (+6 -2) 📝 `tests/support/cli/cliCommandSessionWorker.mjs` (+5 -2) 📝 `tests/support/cli/compiledCliWorker.mjs` (+2 -1) </details> ### 📄 Description  ## Summary Sequential `analyze-javascript-application` calls each started a new Node analysis worker. That process startup, not fixture I/O or the already-shared CLI session, was the large cost in the slowest JavaScript acceptance files. A healthy worker now stays parked on the artifact resource scope (and on one CLI-process scope) until that owner closes. ## Problem and expected behavior CI fast timings from run 39553734979 (`d672cc239`, `test-results` / `test-timings.json`; two commits before the previous main) rank the suite at 6515s across 1898 files. Acceptance, boundary, MCP, and CLI are most of that. The longest files: | Seconds | File | | ---: | --- | | 116.7 | `tests/acceptance/applications/javascriptLoaderReviewBoundaries.test.ts` | | 116.7 | `tests/acceptance/applications/exportedFunctionBindingsCli.test.ts` | | 81.3 | `tests/boundary/cli/clientProfiles.test.ts` | | 61.6 | `tests/acceptance/setup/piClientSetup.test.ts` | | 57.5 | `tests/acceptance/applications/javascriptFactoryCompletion.test.ts` | | 54.4 | `tests/boundary/mcp/analysisViewMcp.test.ts` | | 54.2 | `tests/acceptance/setup/registrationRuntimeEnvironmentCli.test.ts` | | 53.1 | `tests/acceptance/tools/qlogCapture.test.ts` | | 51.8 | `tests/acceptance/applications/javascriptIndirectLoaderOwners.test.ts` | | 51.5 | `tests/acceptance/setup/hermesClientSetup.test.ts` | | 48.9 | `tests/boundary/mcp/callRelationshipAvailability.test.ts` | | 46.5 | `tests/acceptance/process/processFinalization.test.ts` | | 45.9 | `tests/acceptance/applications/mobileDirectoryComponents.test.ts` | | 45.0 | `tests/acceptance/electronFusesCli.test.ts` | | 44.6 | `tests/acceptance/setup/dshClientSetup.test.ts` | A CPU profile of one worker startup (215ms, 185 samples) was `compileSourceTextModule` 23.2%, GC 8.1%, idle 7.6%. The ready handshake alone was about 164–216ms. Bundling the worker and `NODE_COMPILE_CACHE` only moved ready to about 126–140ms and 134ms, so those were not shipped. After the process was warm, per-file semantic work was spread across `domain/javascript` with no single function worth a separate change. Zod parse of the result was 9–28ms. Warm compiled-CLI commands on the loader “returned module” fixture were 405–607ms, with a 242–294ms gap before the first JavaScript file. The shared CLI session was already warm; the worker child was the gap. Expected behavior: a later analysis on the same scope and the same heap/timeout reuses that process. A failed, cancelled, or limit-changed analysis does not. Scope close, session stdin end, and one-shot `process.exit` still stop the process and remove its private root. CLI and MCP results stay the same. A limit change closes the previous worker before the replacement is constructed, and does not replace a worker a concurrent analysis parked during that close. If that close fails, the request fails and the still-live worker stays owned; a replacement is not started. Dispatch cleanup still runs when `serve` or output rendering throws. A failed shutdown stays owned for a retry and is reported before the one-shot process exits. A closed stdout pipe still exits 141. ## Change and scope `createJavaScriptSourceAnalysis` keeps one idle worker per `ArtifactResourceScope`. Reconstruction already passes the caller’s scope, so MCP uses the server scope that `createServer` closes on shutdown. The CLI keeps one process-lived scope, retries failed owners without closing a healthy worker, and `closeCliJavaScriptResources` runs from `scripts/rea.mjs` (including a `finally` around dispatch), the compiled CLI session, and the command session. In-process `javascriptSemanticTraceEvidence` closes that scope in `afterAll`. The CLI shutdown is one shared promise. The scope stays owned until that close succeeds, so the exit callback and the dispatch `finally` observe the same attempt. The exit callback writes a cleanup failure and exits non-zero; a closed stdout pipe still exits 141. Different `heapMb` / `timeoutMs` close the parked process, then start another. If that close fails, checkout throws before constructing a replacement and puts the still-live worker back in the slot when it is empty. The replacement claims the slot only when it is still empty; otherwise the extra process is closed through its own port. Result-budget admission is refreshed from the current parent heap on reuse. Every close hook runs. A close that fails both retained cleanup and a close hook reports both resources and both causes. Follow the [contribution scope](https://github.com/morluto/rea/blob/main/CONTRIBUTING.md#contribution-scope). This stays inside the existing static JavaScript analyzer. It does not add target execution, injection, mutation, or protection recovery. Maintainer scope decision: not applicable. ## Contract and boundary impact - Semantic owner and earliest changed stage: the JavaScript source-analysis worker, owned by the artifact resource scope that already owns the analysis. The change is at worker acquisition, before parsing. - Contribution scope fit and linked maintainer decision: ordinary performance fix inside the existing analyzer. Not applicable. - CLI and MCP/tool-catalog contract: none. Same command, arguments, and result schema. The fixture replay kept a 141018-byte stdout. A failed worker shutdown on process exit is now a non-zero status, except a closed stdout pipe, which stays 141. A limit change whose previous worker cannot be stopped fails the analysis instead of returning success with two owned processes. - Provider, bridge, target-format, or platform compatibility: none. The worker is still the owned Node child. - Evidence, artifact, provenance, or reconstruction contract: unchanged. Factory completion still requires CLI and MCP `evidence_id` parity. - Target selection, authority, upstream behavior, and cleanup details (if applicable): no new authority. The detached worker is still stopped by scope close. Failed cleanup still fails the CLI command via `retryCleanup`. A close hook still runs when other retained cleanup is failing, and both failures stay visible. Later hooks still run when an earlier hook fails. - 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. - [x] 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. ## Evidence and regression coverage - Tests added or updated: worker reuse across one scope, a different heap, and scope close, including that the previous process has exited before the next spawn, that a worker parked during a limit-change close is not replaced, and that a failed close does not start a replacement (`javascriptSourceAnalysisWorker.test.ts`); close hook still runs when retained cleanup fails, both failures are reported, and a later hook still runs when an earlier hook fails (`ArtifactResourceScope.test.ts`); CLI shutdown retries after a failed close and shares one in-flight close (`cliJavaScriptResourceClose.test.ts`); dispatcher reports a failed shutdown as exit 1 and keeps closed-pipe status 141 (`dispatcher.test.ts`); in-process CLI closes the parked worker (`javascriptSemanticTraceEvidence.test.ts`). - Base reproduction or other evidence: same-VM `npm run test:focused` on main’s compile, then on this branch. Loader review 60.28s → 30.77s (124 passed both). Factory completion 34.88s → 21.57s; the 40-scenario test 24482ms → 11595ms. The before runs were second, so disk cache favored the slower baseline. - User-visible CLI/MCP output (if applicable): unchanged on the profiled fixture (stdout 141018 bytes, exit 0). A failed worker shutdown during one-shot exit is now written to stderr and is a non-zero status. A limit change that cannot stop the previous worker fails the analysis. - Remaining proof gaps: `exportedFunctionBindingsCli.test.ts` and `javascriptIndirectLoaderOwners.test.ts` use the same command and were not timed again. Client-profile, setup, qlog, and process-finalization files are a different cost and were not changed. A long MCP session’s child heap was not load-tested; the worker already collects when `heapUsed` exceeds half its limit. Full CI shard wall was not re-measured. `peResources` acceptance timing out at 30s is the same failure on current main and is not changed here. For evidence-bearing changes: - [x] Observed, derived, and inferred claims remain distinguishable. - [x] Artifact identity, source provenance, and failed attempts remain preserved. - [x] Unsupported, incomplete, unavailable, or uncertain outcomes remain visible. ## Validation performed - `npm run test:focused -- tests/acceptance/applications/javascriptLoaderReviewBoundaries.test.ts` — before 60.28s / 124 passed; after 30.77s / 124 passed. - `npm run test:focused -- tests/acceptance/applications/javascriptFactoryCompletion.test.ts` — before 34.88s (24482ms and 7974ms); after 21.57s (11595ms and 7166ms). - `npm run test:focused -- tests/boundary/javascript/javascriptSourceAnalysisWorker.test.ts tests/boundary/cli/cliJavaScriptResourceClose.test.ts tests/boundary/cli/dispatcher.test.ts tests/acceptance/applications/peResources.test.ts` — 21 passed. The new case `does not start a replacement when closing the previous worker fails` passed in 544ms. `peResources` passed locally in 18.9s. - `npm run format:check` — passed (4676 files). - `npm run check:fast` — typecheck and lint passed. - Compiled CLI session on the loader fixture — warm commands 405–607ms before; 546ms then 172–189ms after; session exit 0. - CI shard 7/8 on this head `69e1434b` (run 39585068374, job 119172814208) and on main `c324d31b` (run 39584670117, job 119171892915) both fail only `tests/acceptance/applications/peResources.test.ts` with `Test timed out in 30000ms`. Boundary `tests/boundary/filesystem/peResources.test.ts` passed on both shards (562ms and 463ms). This PR does not change PE inspection. - Real Hopper/Ghidra: not applicable. This path does not call them. ## Compatibility, safety, and release - Breaking changes or migration steps: none. - Real Hopper/Ghidra, browser, or OS coverage: not used. Static JavaScript analysis only, on Linux in this VM. - Package or release metadata impact: none. - Security, privacy, process, or containment review: the worker stays detached and owned. It is not reused after a failed, cancelled, or dirty protocol state. Close still removes the private `rea-provider-` root. One-shot CLI `process.exit` reports a failed worker shutdown and exits non-zero, except a closed stdout pipe, which stays 141. A thrown dispatch still reaches that cleanup. A replaced worker is closed before the next process starts. A failed close does not start that next process. A worker parked during that close stays owned. ## 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 Profile evidence, same VM: - Worker startup, 215ms / 185 samples: `compileSourceTextModule` 23.2%, GC 8.1%, idle 7.6%. Ready handshake about 164–216ms. Per-file work after ready had no single dominant frame. - Warm CLI phase gap before the first file: about 242–294ms before, gone on the second command after reuse (progress at 1ms, `a/package.json` at 45ms). - `javascriptLoaderReviewBoundaries.test.ts`: 60.28s → 30.77s. Steady cases fell under Vitest’s 300ms slow-test print; they were about 420–460ms before. - `javascriptFactoryCompletion.test.ts`: 34.88s → 21.57s (hot test 24482ms → 11595ms). Review follow-up on `2a94716e`: a limit change whose previous worker cannot be stopped fails the request before a replacement starts, and that worker stays owned. Shutdown remains one shared close. A limit change claims the slot only when it is still empty. Later close hooks still run when an earlier hook fails. `peResources` acceptance timeout is the same single failure on main `c324d31b` shard 7/8. It is not fixed in this PR. This branch includes `origin/main` through `5eebe2fd`, which treats omitted `exactOptional` inputs as optional. No product conflict edits were required. Not changed: shard count, cancel-in-progress, workflow concurrency, the shared compiled CLI session from #5676 / #5784, and PE resource tests. - [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. - [ ] User-visible CLI/MCP changes include representative output. - [x] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. No linked issue: this follows measured CI file time after #5799, and the timings above are the report. No user-visible CLI/MCP output change on the successful analysis path, so there is no new sample output. A failed worker shutdown on one-shot exit, and a limit change that cannot stop the previous worker, are error paths.  <div><a href="https://cursor.com/agents/bc-43c3bb7e-31cf-5919-9d0d-4fa10d37dc95?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-43c3bb7e-31cf-5919-9d0d-4fa10d37dc95&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:45:15 +00:00