Fix macOS Whisper runtime packaging#4
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro (Legacy) Run ID: 📒 Files selected for processing (7)
📝 WalkthroughSummary by CodeRabbit
WalkthroughPrepares an arm64 macOS Whisper runtime at build time, bundles it into the app, adds installation and recovery paths to WhisperManager, and adds E2E and unit tests plus post-pack signing and CI preparation steps. ChangesBundled macOS Whisper Runtime
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@e2e/helpers/electron-app.ts`:
- Around line 160-166: The env block that spreads process.env into the Electron
launch (the object containing NODE_ENV, AUTODOC_TEST_REAL_SETUP,
AUTODOC_TEST_USER_DATA_DIR/userDataDir, AUTODOC_SKIP_INSTALL_POLICY and
...(extraEnv ?? {})) can inherit AUTODOC_E2E from the parent, preventing a true
real-setup launch; explicitly set AUTODOC_E2E to '0' in that env object (after
the spread) so it overrides any upstream AUTODOC_E2E, ensuring the real packaged
path runs when AUTODOC_TEST_REAL_SETUP is used.
In `@e2e/README.md`:
- Line 13: The README command can break if the .app path contains spaces; update
the documented command so the AUTODOC_PACKAGED_APP_PATH env assignment value is
quoted (e.g., AUTODOC_PACKAGED_APP_PATH="/abs/path/to/AutoDoc.app") in the
example line that shows the test invocation (the line containing
AUTODOC_RUN_PACKAGED_MACOS_WHISPER_RUNTIME_TEST and AUTODOC_PACKAGED_APP_PATH
used with npm run test:e2e).
In `@scripts/prepare-macos-whisper-runtime.js`:
- Around line 228-241: The prep script currently builds runtimeFiles by
spreading the result of copyMatchingFiles(...) but doesn't check for an empty
result; update the logic in prepare-macos-whisper-runtime.js so you first await
copyMatchingFiles(path.join(ggmlPrefix, 'libexec'), /^libggml.*\.so$/i) into a
variable (e.g., ggmlPlugins), and if ggmlPlugins.length === 0 throw an Error or
call process.exit(1) with a clear message that no GGML backend plugins were
staged; then include ...ggmlPlugins when constructing runtimeFiles so the script
fails early instead of silently shipping without backend plugins (REQUIRED_FILES
and the whisper-cpp --help smoke test remain unchanged).
In `@src/main/index.ts`:
- Line 94: The skipInstalledApplicationPolicy flag currently trusts
AUTODOC_SKIP_INSTALL_POLICY alone; tighten the guard so the bypass is only
allowed in non-production contexts by combining the env check with an explicit
non-production condition (e.g., process.env.NODE_ENV !== 'production' or an
isDev/isTest helper) when initializing skipInstalledApplicationPolicy and when
it is consumed elsewhere (reference the skipInstalledApplicationPolicy variable
and the AUTODOC_SKIP_INSTALL_POLICY env var and the other usage point where the
flag is read). If you need an override for prod, require a distinct explicit
opt-in env (e.g., AUTODOC_ALLOW_SKIP_IN_PROD) and validate that is set before
allowing the bypass.
In `@src/main/services/__tests__/whisper-manager.test.ts`:
- Around line 135-151: The runtime-link-failure classification in
classifyWhisperProbeFailure is not gated by OS; update the function so that the
logic that detects macOS dyld messages (e.g., matching "dyld" / "Library not
loaded" / libwhisper.*.dylib in the probe stderr) only returns
'runtime-link-failure' when process.platform === 'darwin', otherwise return the
existing fallback (e.g., 'failed'); modify the detection branch inside
classifyWhisperProbeFailure to check process.platform before emitting
'runtime-link-failure'.
In `@src/main/services/whisper-manager.ts`:
- Around line 57-62: Update the macOS runtime validation to require the full set
of packaged runtime files instead of only the four cores: add 'libomp.dylib' to
MAC_WHISPER_RUNTIME_EXPECTED_FILES and also require any ggml backend plugins by
matching patterns like /^libggml(-.*)?\.dylib$/ (or explicitly include plugin
names if known); ensure the same updated constant/patterns are used by both
bundled-runtime discovery and extracted-runtime root detection in
whisper-manager.ts so partially staged runtimes with missing libomp or ggml
backend plugins are not treated as complete.
- Around line 1396-1404: The macRuntimeLinkFailure detection (variable
macRuntimeLinkFailure) currently only matches `@rpath` dyld errors against
combinedOutput and err.message; update the regex checks used where
combinedOutput and `${err.message}\n${combinedOutput}` are tested to also match
dyld messages that contain `@executable_path` or `@loader_path` (e.g. add
/@executable_path\/lib(?:whisper|ggml|omp)/i and
/@loader_path\/lib(?:whisper|ggml|omp)/i alternatives alongside the existing
`@rpath` pattern) so that the runtime-link-failure branch triggers for packaged
rewrites as well.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro (Legacy)
Run ID: e5b6060e-1438-4d05-b52e-60c457d9b053
⛔ Files ignored due to path filters (9)
resources/macos-whisper-runtime/arm64/libggml-base.0.dylibis excluded by!**/*.dylibresources/macos-whisper-runtime/arm64/libggml-blas.sois excluded by!**/*.soresources/macos-whisper-runtime/arm64/libggml-cpu-apple_m1.sois excluded by!**/*.soresources/macos-whisper-runtime/arm64/libggml-cpu-apple_m2_m3.sois excluded by!**/*.soresources/macos-whisper-runtime/arm64/libggml-cpu-apple_m4.sois excluded by!**/*.soresources/macos-whisper-runtime/arm64/libggml-metal.sois excluded by!**/*.soresources/macos-whisper-runtime/arm64/libggml.0.dylibis excluded by!**/*.dylibresources/macos-whisper-runtime/arm64/libomp.dylibis excluded by!**/*.dylibresources/macos-whisper-runtime/arm64/libwhisper.1.dylibis excluded by!**/*.dylib
📒 Files selected for processing (13)
.github/workflows/build.ymle2e/README.mde2e/helpers/electron-app.tse2e/macos-packaged-whisper-runtime.spec.tselectron-builder.ymlpackage.jsonresources/macos-whisper-runtime/arm64/whisper-cppscripts/after-pack-mac-privacy.cjsscripts/prepare-macos-whisper-runtime.jssrc/main/index.tssrc/main/services/__tests__/whisper-manager.test.tssrc/main/services/__tests__/whisper-onboarding-install.test.tssrc/main/services/whisper-manager.ts
Summary
This fixes the macOS local speech engine install/runtime failures seen in AD-77 and the AD-65 comment logs by shipping a self-contained arm64 Whisper runtime in the app bundle instead of patching Homebrew binaries on customer machines.
What changed
prepare:macos-whisper-runtimebuild step that stages the arm64whisper-cpp,libwhisper,ggml, backend plugins, andlibompruntime files./opt/homebrew.extraResourcesatContents/Resources/macos-whisper-runtime/arm64.install_name_tool/codesignpatching path.Root cause
The v0.1.23-era install path still depended on customer-machine binary rewriting and/or Homebrew-style runtime layout. On machines without Xcode command line tools,
install_name_toolfailed. On machines with incomplete rewritten runtime links,whisper-cppfailed with dyld@rpath/libwhisper.1.dylibload errors. Even after dylib load-command fixes, Homebrew's GGML build embedded a backend plugin search path that could still load/opt/homebrew/Cellar/ggml/...plugins unless patched.Validation
npm run prepare:macos-whisper-runtimenpm run buildAUTODOC_DISABLE_ASAR_INTEGRITY=1 CSC_IDENTITY_AUTO_DISCOVERY=false npx electron-builder --mac dir --publish never --config.mac.identity=null --config.mac.notarize=falseAUTODOC_RUN_PACKAGED_MACOS_WHISPER_RUNTIME_TEST=1 AUTODOC_PACKAGED_APP_PATH=/Volumes/DuetDrive/Repos/AutoDocLocal/dist/mac-arm64/AutoDoc.app npm run test:e2e -- e2e/macos-packaged-whisper-runtime.spec.tsnpm run test:main:run -- src/main/services/__tests__/whisper-onboarding-install.test.ts src/main/services/__tests__/whisper-manager.test.tsnpm run typecheck:nodegit diff --checkLocal note: the unsigned local mac dir build needed
AUTODOC_DISABLE_ASAR_INTEGRITY=1, matching the repo's existing mac smoke-test note for this volume.