milady: fix desktop update, startup, and legacy onboarding

[Dexploarer]

Summary

• block in-place desktop updates when Milady is running outside /Applications or ~/Applications on macOS
• fix the App.tsx hook-order crash behind minified React error #300
• recover legacy local Eliza installs during startup so existing ~/.eliza users are not bounced back into onboarding
• detect existing desktop/local installs through the native bridge and recognize older agent config shapes during onboarding bootstrap

Testing

• bunx vitest run packages/app-core/src/App.test.ts packages/app-core/src/components/ReleaseCenterView.auto-update.test.ts apps/app/electrobun/src/native/tests/desktop.test.ts apps/app/electrobun/src/tests/kitchen-sink.test.ts
• bunx vitest run apps/app/electrobun/src/native/tests/agent.test.ts packages/app-core/src/state/onboarding-bootstrap.test.ts packages/app-core/src/api/server.onboarding-status.test.ts packages/app-core/src/components/onboarding/WelcomeStep.test.ts packages/app-core/src/App.test.ts
• bun run check
• bun run pre-review:local

Why

• prevents Terminal-launched or DMG-mounted installs from tripping macOS App Management prompts during self-update
• removes a concrete hook-order startup crash in the desktop shell
• fixes the legacy onboarding regression where users with an existing .eliza directory could be forced back into onboarding before the local runtime/config was recognized

[github-actions]

I have enough context. Here's my review:

---

PR #1207 Review — guard desktop updates outside Applications and fix React #300

1. Classification: Bug fix (two independent fixes bundled together)

2. Scope verdict: In scope — both changes address real defects: a crash-class React hook ordering violation and a macOS updater failure mode that triggers App Management permission prompts for non-Applications installs.

3. Code quality: Pass

The implementation is clean and well-structured:

• getUpdaterAvailability() / resolveMacAppBundlePath() are private, well-scoped, and handle the macOS .app path resolution correctly by walking up the directory tree. path.resolve is used throughout, preventing symlink/traversal surprises.
• The canAutoUpdate guard is applied consistently at all three enforcement points: checkForUpdates, applyUpdate, and the background update scheduler in index.ts.
• The buildUpdaterSnapshot default-parameter pattern (availability = this.getUpdaterAvailability()) avoids redundant calls at callers that already have the value. Fine.
• The App.tsx fix is the minimal correct change: moving the startupError early return to after the useState/useRef/useEffect calls resolves the hook-order violation. No excess refactoring.
• One minor nit: the user-facing disabled reason says "Move Milady.app to /Applications..." but the code also accepts ~/Applications. The message could be slightly more accurate ("move to /Applications or ~/Applications") but it's not a blocker — /Applications is the canonical answer for most users.

4. Security: Clear

• No new dependencies.
• No credentials or secrets.
• The availability check is read-only (process.execPath, process.platform, path.resolve). No injection vectors.
• Moving Updater.applyUpdate() behind desktop.applyUpdate() actually tightens the security posture — the guard now applies even if a future caller bypasses the check-for-updates flow.

5. Tests: Adequate

Regression tests are present for both bugs:

• desktop.test.ts: 3 new cases covering the "outside Applications" state snapshot, the blocked applyUpdate, and the allowed applyUpdate.
• App.test.ts: 1 new case explicitly exercising the hook-order scenario (mount clean → inject startupError → re-render, verify StartupFailureView renders without crashing).
• ReleaseCenterView.auto-update.test.ts: 1 new case verifying that both action buttons are disabled and the warning text is surfaced when canAutoUpdate: false.
• Mock updates to kitchen-sink.test.ts align the existing mock shape with the new schema fields.

6. Decision: APPROVE

Both fixes are correct, minimal, and backed by targeted regression tests. The desktop update guard addresses a real macOS behavioural failure (App Management prompts / update corruption from DMG-mounted installs), and the hook fix resolves a React invariant violation that could crash the app on certain startup-error transitions. No concerns.

[github-actions]

I have a complete picture of the PR. Let me deliver the review.

---

1. Classification: bug fix (three distinct bugs: macOS update guard, React #300 hook-order crash, legacy onboarding regression)

2. Scope verdict: in scope — all three fixes address concrete crash/regression issues with the desktop shell and onboarding flow

3. Code quality: pass

Notable points:

• resolveMacAppBundlePath while(true) loop terminates correctly at filesystem root (parent === current). No infinite-loop risk.
• The path-prefix check in getUpdaterAvailability uses path.sep-suffixed root comparison, correctly preventing /Applications-extra/Foo.app from being accepted as valid.
• applyUpdate() calling Updater.applyUpdate() without await is intentional — it's a fire-and-forget that restarts the app, consistent with the original handler.
• The forceLocalBootstrapRef pattern (ref, not state) is the right choice: avoids a re-render cycle and survives closure captures.
• detectExistingOnboardingConnection timeout via Promise.race + Symbol sentinel is clean and correct.
• AppContext.tsx now unconditionally calls agentStart in local mode (previously guarded by absence of API base). Author's comment says it's idempotent — reasonable given the embedded agent start is designed for repeated calls.
• buildUpdaterSnapshot(undefined, availability) avoids a redundant getUpdaterAvailability() call inside the snapshot builder — subtle but correct.
• Arrow wrapper () => handleOnboardingUseLocalBackend() in WelcomeStep.tsx is unnecessarily verbose but harmless.
• onboarding.customSetup is confirmed to exist pre-PR in all six locale files. No missing translation.

4. Security: clear

• inspectExistingElizaInstall reads filesystem paths derived from env vars (MILADY_CONFIG_PATH, ELIZA_CONFIG_PATH, MILADY_STATE_DIR, ELIZA_STATE_DIR). These are process-level env vars, not user-controlled input. Result stays in the local IPC bridge (Electrobun → trusted webview), not exposed externally.
• getConfig() probe reads local agent config; no credentials are logged or forwarded, only structure presence is checked.
• No new npm dependencies. No new postinstall scripts. No secrets in code.

5. Tests: adequate

Regression tests present and targeted for all three bugs:

• App.test.ts — verifies hook order is stable when startupError appears post-mount (exact regression for feat: plugin eject system — edit upstream plugin source locally #300)
• desktop.test.ts — verifies blocks outside Applications, allows from Applications, correct getUpdaterState shape
• ReleaseCenterView.auto-update.test.ts — verifies buttons are disabled and warning renders when canAutoUpdate: false
• onboarding-bootstrap.test.ts — 6 cases covering all code paths in detectExistingOnboardingConnection and deriveDetectedProviderPrefill
• WelcomeStep.test.ts — both detected/not-detected UI branches
• server.onboarding-status.test.ts — legacy workspace config shape recognized as complete
• agent.test.ts — 3 cases for inspectExistingElizaInstall covering config-path-env, state-dir-env, and default-state-dir sources

6. Decision: APPROVE

Three real bugs, three targeted fixes, comprehensive test coverage, clean implementation. No issues warrant blocking.

[github-actions]

I've reviewed all 37 changed files. Here's my assessment:

---

PR #1207 Review — fix desktop update, startup, and legacy onboarding

1. Classification

Bug fix (three distinct crashes/regressions bundled together)

2. Scope Verdict

In scope. All three fixes address real user-facing breakage: macOS App Management prompt loops during self-update, a concrete React hook-order crash (minified Error #300), and users with existing ~/.eliza installs being dropped back into onboarding.

3. Code Quality

Pass.

• resolveMacAppBundlePath in desktop.ts: straightforward upward path walk; correctly handles edge cases by returning null when no .app boundary is found.
• getUpdaterAvailability checks against both /Applications and ~/Applications, uses path.resolve consistently — no path traversal risk.
• Hook-order fix in App.tsx is correct: the startupError early-return was firing before useState/useRef calls, violating the Rules of Hooks. Moving it after hook declarations is the right fix.
• detectExistingOnboardingConnection uses a Promise.race timeout pattern correctly and clears the timeout to avoid leaks.
• ExistingElizaInstallInfo is defined in three layers (agent.ts, rpc-schema.ts, electrobun-rpc.ts) — expected cross-boundary duplication, not a problem.
• Minor: applyDetectedProviders has an empty [] dep array while calling four useState setters, which is correct (setters are stable) but linters sometimes flag it. Not a bug.
• Formatting-only changes in CharacterEditor.tsx, AdvancedPageView.tsx, useVoiceChat.ts are Biome-compliant cleanups — fine to include.

4. Security

Clear.

• No new dependencies added.
• inspectExistingElizaInstall does synchronous fs reads on the main process thread, but only for existence/listing checks. No data is transmitted externally.
• No auth, permissions, or credential handling changes.
• No prompt injection surface introduced.

5. Tests

Adequate.

Every fix has a regression test:

• desktop.test.ts: 3 new cases covering the macOS /Applications guard (reports unavailable, blocks apply, allows from correct path).
• App.test.ts (new): validates hook order stays stable when startupError appears after initial mount.
• agent.test.ts: 3 cases for inspectExistingElizaInstall (default ~/.eliza, env override, explicit config path).
• onboarding-bootstrap.test.ts (new): 6+ cases for detectExistingOnboardingConnection and deriveDetectedProviderPrefill, including the legacy agents.defaults.workspace shape.
• server.onboarding-status.test.ts: legacy workspace config roundtrip against the real API server.
• ReleaseCenterView.auto-update.test.ts (new): UI guard disables update buttons when canAutoUpdate: false.
• WelcomeStep.test.ts (new): verifies both branches of the existing-install detection UI.

6. Decision

APPROVE

All three bugs are well-diagnosed, fixed correctly, and covered by targeted regression tests. The implementation is clean with no security concerns or scope creep. Legendary contributor with a pattern that matches the described fixes end-to-end.

[github-actions]

The GitHub commands require interactive approval. Here is the complete review for posting:

---

Review: PR #1207 — fix desktop update, startup, and legacy onboarding

1. Classification: Bug fix (four distinct crashes/regressions)

2. Scope verdict: In scope

3. Code quality: Pass

Four distinct fixes, each tightly scoped:

• desktop.ts / index.ts — macOS update guard: getUpdaterAvailability() walks up process.execPath to find the .app bundle and rejects canAutoUpdate if the bundle sits outside /Applications or ~/Applications. resolveMacAppBundlePath is correct — the loop terminates at the filesystem root. applyUpdate() now throws a user-readable message instead of silently triggering a broken self-update from a DMG mount. The guard also runs before the network check in setupUpdater, so no unnecessary requests are made.

• App.tsx — React hook ordering (error feat: plugin eject system — edit upstream plugin source locally #300): The original if (startupError) { return <StartupFailureView /> } appeared before useState(false) and useRef(true), violating rules of hooks. Moving it after those calls is the correct fix. App.test.ts directly exercises the late-state error path.

• agent.ts — inspectExistingElizaInstall: Clean candidate-priority logic (config-path-env → state-dir-env → default ~/.eliza). Deduplication via findIndex is correct. The fallback to candidates[0] handles the empty-env case properly. listStateEntries silently returns [] on read failure — appropriate for a probe function.

• onboarding-bootstrap.ts / WelcomeStep.tsx — legacy onboarding recovery: Detects existing installs through the native bridge and short-circuits the onboarding wizard. The detectExistingOnboardingConnection path is conservative — only asserts detectedExistingInstall when the API is reachable and reports complete.

Minor notes (non-blocking):

• ExistingElizaInstallInfo is defined in three places (agent.ts, rpc-schema.ts, bridge/electrobun-rpc.ts). Structurally necessary for the RPC boundary but worth consolidating if the interface evolves.
• Large diffs in CharacterEditor.tsx, MediaGalleryView.tsx, and VrmTeleportEffect.ts are Biome reformatting. The one functional change in CharacterEditor.tsx removes (currentCharacter as any) casts — correct improvement.

4. Security: Clear — no new attack surfaces, no credential handling changes, no new dependencies, no postinstall scripts.

5. Tests: Adequate

Every fix has direct regression coverage: desktop.test.ts (3 new tests), agent.test.ts (3 new tests), App.test.ts (new file), server.onboarding-status.test.ts (legacy config shape), onboarding-bootstrap.test.ts (new file, 132 lines), WelcomeStep.test.ts (new file), ReleaseCenterView.auto-update.test.ts (new file).

Decision: APPROVE