Skip to content

fix(dev): rebuild incomplete macOS Electron caches - #2657

Closed
rudycelekli wants to merge 4 commits into
debpalash:mainfrom
rudycelekli:fix/mac-development-cache-recovery-20261006
Closed

rudycelekli wants to merge 4 commits into
debpalash:mainfrom
rudycelekli:fix/mac-development-cache-recovery-20261006

Conversation

@rudycelekli

@rudycelekli rudycelekli commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Summary

If the cached branded macOS development app loses its executable, the cache fails validation but still occupies its target directory. Staging a rebuilt bundle into that occupied path fails repeatedly. Invalid cache contents should be removed before the validated replacement is published.

Closes #2656.

Changes

  • Remove invalid cache contents before publishing a rebuilt development bundle.
  • Hold classic file-and-command macOS lockf custody throughout preparation; recover only demonstrably dead same-host owners and retire the entire custody process group.
  • Synchronize relevant documentation and add a quiet credited Unreleased changelog entry.
  • No new UI strings or locale keys; maintained version files are unchanged.

Type

  • 🐛 Bug fix
  • ✨ New feature
  • ♻️ Refactor
  • 📝 Documentation
  • 🧪 Tests
  • 🔧 CI / Build
  • 🚀 Release prep

Testing

  • Native macOS bundle, concurrent repair, interrupted-owner and launcher tests: 9 passed with Node24.18.0, reused Vitest5.0.1 and declared Electron44.3.0/electron-vite6.0.0-beta.1 packages; no Electron binary download. The repository pins Vitest4.1.11.
  • Native compatibility repetition: 9 passed, with interrupted workers and independent contenders using compiled official Apple shell_cmds-278 lockf; the preceding descriptor-only source fails the actual worker-entry regression and the old command returns usage64. Tested on the current macOS kernel, not an older-OS matrix.
  • Actual competing lockf acquisition is denied while a producer lives. After its observed SIGKILL both holder-group/helper PIDs retire; retries recover incomplete contents and preserve an already published executable inode. Live/unknown ownership remains untouched.
  • git diff --check and changelog/locale/CJK/version checks: 589 passed, one unchanged environment failure because this Python runtime lacks installed omnivoice package metadata.
  • Full bun run check:electron was attempted; broader typecheck remains blocked by missing workspace UI/build dependencies and existing type errors. Full backend and hosted platform gates are not claimed green from these local tests.
  • Human CLA signature was already registered; its current hosted context and all other hosted checks must be evaluated separately at the new head. No legal attestation was submitted by this maintenance work.

Checklist

  • I've tested this locally
  • Every commit author has signed the CLA (the CLA check tells you how)
  • I've updated relevant documentation (if applicable)
  • No local machine paths, logs, or personal env details in this PR
  • Maintained version files are in sync (if an owner-requested bump): root package.json, pyproject.toml, backend/core/version.py, and lockfiles
  • If this PR changes runtime behavior, the regression fixture at tests/fixtures/omnivoice_data/ still loads green on the smoke-matrix CI job (macOS + Windows + Linux)

Release cadence

VoiceStudio ships continuous-to-main — no release candidates, no soak windows.
Every merged PR is immediately part of rolling source (main) and Docker
:latest. Electron artifact rehearsals validate desktop packages without publishing.
Version bumps require owner approval; validated releases are tagged from main
and published explicitly under the release checklist.
Users who want stability install an Electron release or pin Docker :stable.

The macOS development launcher removes an invalid cached bundle before publishing a replacement and uses lockf custody to coordinate launchers and recover only demonstrably abandoned locks. This prevents incomplete cache contents from blocking rebuilds. Review the macOS lock-recovery paths; full-suite and platform checks were not completed.

Maintenance verification — October 6

Actual child-process regressions kill an observed launcher before retirement and after publication, verify recovery/retained executable inode, and preserve live/unknown owners; the existing concurrent repair proof also passes. The unchanged-head interruption regression fails before the fix. All nine launcher tests pass afterward with Node 24.18.0, existing Vitest 5.0.1 and the declared Electron/electron-vite packages (binary downloads skipped).

Full check:electron was attempted and retried, but the broader workspace typecheck remains blocked by absent UI/build dependencies and unrelated existing type errors. Mechanical checks report 589 passed and one missing-installed-package-metadata environment failure. Full hosted platform gates remain separate; no full-suite green claim.

macOS lockf compatibility correction

Maintenance head a3ccce0 replaces the recent descriptor-only interface with classic persistent-file custody. Official Apple macOS13.0 source and compiled native command controls support the correction; current-kernel testing does not qualify a full older-macOS matrix. See Testing for exact scope and remaining environment gates.

Signed-off-by: Rudy Celekli <47457359+rudycelekli@users.noreply.github.com>
@coderabbitai

coderabbitai Bot commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: debpalash/VoiceStudio/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 28331dc4-f299-4292-960d-87ec79863728
📥 Commits

Reviewing files that changed from the base of the PR and between 59f5cf8 and a3ccce0.

📒 Files selected for processing (4)
  • CHANGELOG.md
  • docs/electron-repair.md
  • electron/scripts/dev.mjs
  • electron/src/main/dev-bundle.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • CHANGELOG.md
  • docs/electron-repair.md

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.


📝 Walkthrough

Walkthrough

The macOS development launcher validates and rebuilds its Electron cache under a custody lock. The lock records ownership and reclaims a lock directory only when a same-host process is confirmed dead. Integration tests cover cache reuse, rebuilding, concurrent launchers, and lock recovery.

Changes

macOS Development Cache Recovery

Layer / File(s) Summary
Custody lock handling
electron/scripts/dev.mjs
Adds a lockf custody lock with a 60-second limit and owner metadata. It reclaims a lock directory only when its same-host owner is confirmed dead.
Protected cache lifecycle and validation
electron/scripts/dev.mjs, electron/src/main/dev-bundle.test.ts, CHANGELOG.md, docs/electron-repair.md
Runs cache validation and rebuilding under the lock. macOS tests cover reuse, rebuilding, concurrent publication, and owner recovery. The changelog and documentation describe cache repair and lock handling.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix · Severity of issue fixed: Low

Suggested reviewers: debpalash

Merge Risk: ⚪ Minimal · up to a3ccc

The cache repair is ready to merge after normal platform checks. The reviewed paths do not establish a remaining failure that blocks rebuilding or permits competing publication.

🚥 Pre-merge checks | ✅ 7 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files. (2 skipped: 2 … Write docstrings for the functions missing them to satisfy the coverage threshold.
Cross-Platform Default Parity ⚠️ Warning The PR changes default behavior on macOS only. launchElectronVite calls the cache repair and custody code automatically when process.platform === 'darwin' (electron/scripts/dev.mjs:269–280); the c… Make the behavior equivalent by default on macOS, Windows, and Linux, or put the macOS-only cache repair and locking behind an explicit opt-in such as an environment variable or CLI flag.
✅ Passed checks (7 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Issue #2656 requires invalid branded macOS cache contents to be removed before replacement publication. prepareMacDevElectron removes the invalid destination under cache custody before staging, and …
Out of Scope Changes check ✅ Passed The lockf coordination, recovery tests, repair documentation, and credited changelog entry support the cache-rebuild and safe-publication objective in #2656. The reviewed changes show no unrelated w…
I18n Completeness (21 Locales) ✅ Passed The PR changes only the Electron development launcher and its main-process tests; it does not change Electron UI code or locale catalogs. The added strings are launcher errors and test diagnostics, no…
Local-First Guarantee ✅ Passed The PR introduces no required cloud call, account, API key, or outbound request. The changed launcher code uses local filesystem operations, process signals, hostname(), lockf, and local macOS bun…
Backward Compatibility ✅ Passed The pull request changes only the Electron development launcher, its tests, and documentation/changelog. The code rebuilds a branded Electron bundle in `electron/node_modules/.cache/voicestudio-electr…
Title check ✅ Passed The title uses the required conventional-commit format with the dev scope and describes the cache repair. The description references issue #2656.
Description check ✅ Passed The description includes the required Summary, Changes, Type, Testing, Checklist, and Release cadence sections. It explains the change, reports test results and limitations, and identifies unchecked p…
Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files. (2 skipped: 2 unsupported.)

Full details: Cross-Platform Default Parity

Explanation

The PR changes default behavior on macOS only. launchElectronVite calls the cache repair and custody code automatically when process.platform === 'darwin' (electron/scripts/dev.mjs:269–280); the changed prepareMacDevElectron path now locks and rebuilds the cache (lines 195–206). Windows and Linux do not use this path, and no explicit opt-in is provided. This is a platform-divergent default under the check.

  • Fix all pre-merge checks with AI
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@greptile-apps

greptile-apps Bot commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Retrigger

[High risk] Development launcher adds macOS cache locking and rebuild logic.

The PR appears safe to merge based on the reviewed changes.

Summary

The PR repairs incomplete macOS development bundles and replaces the invalid custody invocation with a file-and-command lockf holder. It also adds native contention and interruption tests and updates the documentation and changelog.

Reviews (4) · Last reviewed commit: "fix: retain macOS cache custody with cla..."

Comment thread electron/scripts/dev.mjs Outdated
@github-actions

github-actions Bot commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

All contributors on this pull request have signed the VoiceStudio CLA. Thank you!

Signed-off-by: Rudy Celekli <47457359+rudycelekli@users.noreply.github.com>
Comment thread electron/scripts/dev.mjs Outdated
Signed-off-by: Rudy Celekli <rudy@gradiahq.com>
Comment thread electron/scripts/dev.mjs Outdated
Signed-off-by: Rudy Celekli <47457359+rudycelekli@users.noreply.github.com>
@debpalash

Copy link
Copy Markdown
Owner

Thanks for the report and the work on this, @rudycelekli. The problem in #2656 is fixed on main by b5cf3cd0 (merged in #2673), so I'm closing this pull request as already fixed. It will ship in v0.5.7. Reporter credit stays in the changelog.

@debpalash debpalash closed this Oct 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Incomplete branded macOS Electron development cache prevents rebuilding

2 participants