[PR #5784] [MERGED] perf(test): reuse one compiled CLI for the remaining hot command files #7058

Closed
opened 2026-11-01 21:45:02 +00:00 by bsakaguchi · 0 comments
Owner

📋 Pull Request Information

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

Base: main ← Head: cursor/ci-queue-shard-wins-d342


📝 Commits (1)

  • b98f734 perf(test): reuse one compiled CLI for the remaining hot command files

📊 Changes

6 files changed (+98 additions, -73 deletions)

View changed files

📝 docs/testing.md (+7 -2)
📝 tests/acceptance/applications/applicationWorkflowCli.test.ts (+25 -23)
📝 tests/acceptance/goBinaryCli.test.ts (+16 -16)
📝 tests/acceptance/il2cppMetadataCli.test.ts (+16 -16)
📝 tests/boundary/cli/dylibResolutionCli.test.ts (+20 -16)
📝 tests/support/cli/cliFixture.ts (+14 -0)

📄 Description

Summary

The slowest remaining acceptance commands still started a compiled CLI process for every call, after the shared JavaScript session (#5676) and the warm export-binding CLI (#5679). Go binary, IL2CPP metadata, dylib resolution, and application-workflow JSON commands now reuse one compiled process per file. Assertions are unchanged.

Human owner: @morluto.

Problem and expected behavior

On fast main run 39542179402 (3a0e91dd, after #5676 and before #5679), these files were still among the longest built modules: goBinaryCli.test.ts 76.0s, applicationWorkflowCli.test.ts 69.5s, il2cppMetadataCli.test.ts 42.9s, and dylibResolutionCli.test.ts 40.9s. Each cli.run or execFile(scripts/rea.mjs) paid process startup again. Commands with stdin or an explicit deadline still need a fresh process. These calls have neither.

Change and scope

runFileCli sends a cliTest command through the existing file-scoped session. Go, IL2CPP, and dylib-resolution tests use it. Application workflow keeps its own session so a non-zero status whose stdout is JSON is still returned as that document, matching the previous execFile catch path. The session restarts when the environment changes, after 32 commands, or at a 1 GiB resident set, and the file closes it afterward.

Not included: shard-count changes, cancel-in-progress on main, or moving setup commands that pass timeoutMs. Eight fast shards already sit on the ten-minute job budget (slowest built shard execution on that run was 547s). Main keeps the in-progress feedback run so a burst of pushes cannot cancel every attempt. Vitest already forces maxWorkers: 1 when fileParallelism is false, so the process-boundary lane is already serial in the sequencer.

No separate issue is linked. This continues the standing CI wall-time work already landed in #5676 and #5679.

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

Contract and boundary impact

  • Semantic owner and earliest changed stage: the acceptance and boundary tests own the reused 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 same JSON results are still required.
  • Target selection, authority, upstream behavior, and cleanup details (if applicable): the shared child is closed with the file. A different environment, the command limit, or the resident-set limit starts another owned process. Stdin and explicit deadlines stay on a fresh process.
  • 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: the four files above call the shared session. No assertions were removed. tests/support/cli/cliFixture.ts adds runFileCli. docs/testing.md records both reuse paths.
  • Base reproduction or other evidence: warm local vitest runs on Node 24.18.0, no coverage. Before: Go 39.65s, application workflow 46.08s, IL2CPP 35.32s, dylib resolution 21.15s. After: 14.15s, 21.01s, 20.38s, and 2.17s. All four passed before and after. tests/boundary/cli/sharedJavaScriptCliSession.test.ts passed.
  • User-visible CLI/MCP output (if applicable): none. There is no public CLI or MCP projection change, so no representative command output is included.
  • Remaining proof gaps: GitHub Actions has not run this head. The local ratio is not a measured CI shard wall time. These files share eight shards and two workers, so the uncontended shard critical path should drop by tens of seconds of module time, not by the sum of the file times. Runner queue is a separate, larger delay and is unchanged.

Validation performed

  • node node_modules/vitest/vitest.mjs run --project acceptance tests/acceptance/goBinaryCli.test.ts — passed, warm 39.65s then 14.15s.
  • node node_modules/vitest/vitest.mjs run --project acceptance tests/acceptance/applications/applicationWorkflowCli.test.ts — passed, warm 46.08s then 21.01s.
  • node node_modules/vitest/vitest.mjs run --project acceptance tests/acceptance/il2cppMetadataCli.test.ts — passed, 35.32s then 20.38s.
  • node node_modules/vitest/vitest.mjs run --project cli-boundary tests/boundary/cli/dylibResolutionCli.test.ts — passed, 21.15s then 2.17s.
  • node node_modules/vitest/vitest.mjs run --project cli-boundary tests/boundary/cli/sharedJavaScriptCliSession.test.ts — passed.
  • npm run check:fast — passed (typecheck and lint, 24.2s).

Compatibility, safety, and release

  • Breaking changes or migration steps: none.
  • Real Hopper/Ghidra, browser, or OS coverage: not applicable. These tests use local fixtures and the compiled CLI.
  • Package or release metadata impact: none.
  • Security, privacy, process, or containment review: the test child is still owned and closed with the file. No new product 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.
  • 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.
Open in Web Open in Cursor 

🔄 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/5784 **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/ci-queue-shard-wins-d342` --- ### 📝 Commits (1) - [`b98f734`](https://github.com/morluto/rea/commit/b98f734517e0662d0a26f02c40df66da08f59cb6) perf(test): reuse one compiled CLI for the remaining hot command files ### 📊 Changes **6 files changed** (+98 additions, -73 deletions) <details> <summary>View changed files</summary> 📝 `docs/testing.md` (+7 -2) 📝 `tests/acceptance/applications/applicationWorkflowCli.test.ts` (+25 -23) 📝 `tests/acceptance/goBinaryCli.test.ts` (+16 -16) 📝 `tests/acceptance/il2cppMetadataCli.test.ts` (+16 -16) 📝 `tests/boundary/cli/dylibResolutionCli.test.ts` (+20 -16) 📝 `tests/support/cli/cliFixture.ts` (+14 -0) </details> ### 📄 Description <!-- CURSOR_AGENT_PR_BODY_BEGIN --> ## Summary The slowest remaining acceptance commands still started a compiled CLI process for every call, after the shared JavaScript session (#5676) and the warm export-binding CLI (#5679). Go binary, IL2CPP metadata, dylib resolution, and application-workflow JSON commands now reuse one compiled process per file. Assertions are unchanged. Human owner: @morluto. ## Problem and expected behavior On fast main run `39542179402` (`3a0e91dd`, after #5676 and before #5679), these files were still among the longest built modules: `goBinaryCli.test.ts` 76.0s, `applicationWorkflowCli.test.ts` 69.5s, `il2cppMetadataCli.test.ts` 42.9s, and `dylibResolutionCli.test.ts` 40.9s. Each `cli.run` or `execFile(scripts/rea.mjs)` paid process startup again. Commands with stdin or an explicit deadline still need a fresh process. These calls have neither. ## Change and scope `runFileCli` sends a `cliTest` command through the existing file-scoped session. Go, IL2CPP, and dylib-resolution tests use it. Application workflow keeps its own session so a non-zero status whose stdout is JSON is still returned as that document, matching the previous `execFile` catch path. The session restarts when the environment changes, after 32 commands, or at a 1 GiB resident set, and the file closes it afterward. Not included: shard-count changes, cancel-in-progress on main, or moving setup commands that pass `timeoutMs`. Eight fast shards already sit on the ten-minute job budget (slowest built shard execution on that run was 547s). Main keeps the in-progress feedback run so a burst of pushes cannot cancel every attempt. Vitest already forces `maxWorkers: 1` when `fileParallelism` is false, so the process-boundary lane is already serial in the sequencer. No separate issue is linked. This continues the standing CI wall-time work already landed in #5676 and #5679. Contribution scope: not applicable. This is test harness work inside existing CLI/MCP checks. ## Contract and boundary impact - Semantic owner and earliest changed stage: the acceptance and boundary tests own the reused 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 same JSON results are still required. - Target selection, authority, upstream behavior, and cleanup details (if applicable): the shared child is closed with the file. A different environment, the command limit, or the resident-set limit starts another owned process. Stdin and explicit deadlines stay on a fresh process. - 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: the four files above call the shared session. No assertions were removed. `tests/support/cli/cliFixture.ts` adds `runFileCli`. `docs/testing.md` records both reuse paths. - Base reproduction or other evidence: warm local vitest runs on Node 24.18.0, no coverage. Before: Go 39.65s, application workflow 46.08s, IL2CPP 35.32s, dylib resolution 21.15s. After: 14.15s, 21.01s, 20.38s, and 2.17s. All four passed before and after. `tests/boundary/cli/sharedJavaScriptCliSession.test.ts` passed. - User-visible CLI/MCP output (if applicable): none. There is no public CLI or MCP projection change, so no representative command output is included. - Remaining proof gaps: GitHub Actions has not run this head. The local ratio is not a measured CI shard wall time. These files share eight shards and two workers, so the uncontended shard critical path should drop by tens of seconds of module time, not by the sum of the file times. Runner queue is a separate, larger delay and is unchanged. ## Validation performed - `node node_modules/vitest/vitest.mjs run --project acceptance tests/acceptance/goBinaryCli.test.ts` — passed, warm 39.65s then 14.15s. - `node node_modules/vitest/vitest.mjs run --project acceptance tests/acceptance/applications/applicationWorkflowCli.test.ts` — passed, warm 46.08s then 21.01s. - `node node_modules/vitest/vitest.mjs run --project acceptance tests/acceptance/il2cppMetadataCli.test.ts` — passed, 35.32s then 20.38s. - `node node_modules/vitest/vitest.mjs run --project cli-boundary tests/boundary/cli/dylibResolutionCli.test.ts` — passed, 21.15s then 2.17s. - `node node_modules/vitest/vitest.mjs run --project cli-boundary tests/boundary/cli/sharedJavaScriptCliSession.test.ts` — passed. - `npm run check:fast` — passed (typecheck and lint, 24.2s). ## Compatibility, safety, and release - Breaking changes or migration steps: none. - Real Hopper/Ghidra, browser, or OS coverage: not applicable. These tests use local fixtures and the compiled CLI. - Package or release metadata impact: none. - Security, privacy, process, or containment review: the test child is still owned and closed with the file. No new product 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. - [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. <!-- CURSOR_AGENT_PR_BODY_END --> <div><a href="https://cursor.com/agents/bc-ac6d4d10-cdbc-5a68-a6b1-e326f171d342?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>&nbsp;<a href="https://cursor.com/background-agent?bcId=bc-ac6d4d10-cdbc-5a68-a6b1-e326f171d342&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>&nbsp;</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:02 +00:00
Sign in to join this conversation.
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#7058
No description provided.