Skip to content

Feat/cli editor cache#749

Closed
IvanMurzak wants to merge 4 commits into
mainfrom
feat/cli-editor-cache
Closed

Feat/cli editor cache#749
IvanMurzak wants to merge 4 commits into
mainfrom
feat/cli-editor-cache

Conversation

@IvanMurzak
Copy link
Copy Markdown
Owner

This pull request introduces a persistent disk cache for resolved Unity Editor binary paths, significantly improving the performance of repeated editor launches by avoiding redundant and slow Unity Hub lookups. It also improves error reporting for Unity Hub CLI failures and adds comprehensive unit and integration tests for the new caching logic.

Key changes include:

Editor Path Caching

  • Added a new editor-cache.ts module that provides functions to read, write, and clear cached Unity Editor binary paths per version, using a JSON file in the user's home directory. The cache self-evicts stale entries if the binary no longer exists. (cli/src/utils/editor-cache.ts)
  • Integrated the cache into findEditorPath in unity-editor.ts, so cache hits bypass slow Unity Hub lookups, and resolved paths are stored for future use. (cli/src/utils/unity-editor.ts) [1] [2] [3] [4] [5]
  • On editor spawn failures, the cache entry is cleared to avoid repeated failures with a broken path. (cli/src/lib/open.ts) [1] [2]

Error Handling and Diagnostics

  • Improved error reporting for Unity Hub CLI invocations by capturing and displaying stderr output on failures, making troubleshooting easier. (cli/src/utils/unity-hub.ts) [1] [2] [3] [4]

Testing

  • Added a thorough unit test suite for the editor cache, covering read, write, eviction, and key separation logic. (cli/tests/editor-cache.test.ts)
  • Added an integration test to ensure findEditorPath returns cached results without invoking Unity Hub helpers when the cache is warm. (cli/tests/unity-editor.test.ts) [1] [2]

These changes collectively make repeated Unity Editor launches much faster, improve reliability, and provide better diagnostics for users.

Two CLI quality-of-life improvements for `unity-mcp-cli open`:

1. Editor-path cache
   - New `cli/src/utils/editor-cache.ts` persists the resolved Unity
     Editor binary path to `~/.unity-mcp-cli-editor-cache.json`,
     keyed by Unity version (with an `__auto__` slot for the
     no-version case). Same convention as the existing
     `~/.unity-mcp-cli-update.json`.
   - `findEditorPath` (`utils/unity-editor.ts`) checks the cache
     first and returns immediately on hit, skipping the Unity Hub
     Electron probe entirely. Every successful resolution writes
     back to the cache (fast path, hub-resolved, and the highest-
     installed fallback).
   - Stale entries auto-evict on read when the cached path no
     longer exists on disk; corrupted-but-existing binaries are
     cleared by an `onError` hook in `openProject` (`lib/open.ts`)
     so the next invocation re-resolves from scratch.
   - Measured impact on Windows with the editor installed at a
     non-default location (`C:\UnityEditor\<version>\Editor\Unity.exe`):
     cold run ~13.0s -> warm runs ~1.2s (~10x speedup; Unity Hub
     CLI invocation eliminated on the warm path).

2. Quiet Unity Hub stderr
   - `listInstalledEditors` and `listAvailableReleases`
     (`utils/unity-hub.ts`) now pipe stderr via
     `stdio: ['ignore', 'pipe', 'pipe']` instead of inheriting it.
     Unity Hub is an Electron app and its embedded Chromium
     routinely logs benign `quota_database.cc` errors to stderr
     which previously leaked into the terminal during a successful
     editor probe. Captured stderr is appended to the surfaced
     error message in the catch path, so real Hub failures stay
     diagnosable.

Full CLI test suite (`npm test` under `cli/`) passes: 370 passed,
1 skipped (pre-existing).
Review cleanup on top of the editor-cache + Hub-stderr commit:

- `utils/unity-hub.ts`: hoist the duplicated stderr-extraction block
  from `listInstalledEditors` and `listAvailableReleases` catch
  handlers into a single `formatExecError(err)` helper. Both catch
  blocks now call it; behaviour unchanged.
- `utils/editor-cache.ts`: trim the AUTO_KEY comment to just the
  why (collision-avoidance with the Unity-version namespace).
- `utils/unity-editor.ts`: tighten the cache-lookup preamble in
  `findEditorPath` from 5 lines to 2.

`npm run build` + `npm test` under `cli/`: 370 passed, 1 skipped.
Address PR #748 review comments:

- `utils/editor-cache.ts`: switch the in-memory cache dict to a
  null-prototype object via `Object.create(null)`, and copy parsed
  JSON entries through that null-prototype shell. Defends against
  a `m_EditorVersion: __proto__` payload (or `--unity __proto__`)
  polluting the cache object's prototype chain. Public API
  unchanged.
- `tests/editor-cache.test.ts` (NEW, 9 tests): cover
  read-missing → null, write/read round-trip, stale-entry
  auto-eviction, versioned-vs-`__auto__` key isolation, and
  `clearCachedEditorPath` precision.
- `tests/unity-editor.test.ts` (+1 test): assert `findEditorPath`
  short-circuits via the cache without calling `ensureUnityHub`
  or `listInstalledEditors`.

`npm test` under `cli/`: 380 passed, 1 skipped.
@IvanMurzak IvanMurzak self-assigned this May 11, 2026
Copilot AI review requested due to automatic review settings May 11, 2026 03:29
@IvanMurzak IvanMurzak added the enhancement New feature or request label May 11, 2026
@IvanMurzak IvanMurzak closed this May 11, 2026
@IvanMurzak IvanMurzak deleted the feat/cli-editor-cache branch May 11, 2026 03:29
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants