Skip to content

fix(desktop): do not double-quote updateMutex path in withRemoteUpdateMutex - #102907

Closed
kyoji2 wants to merge 1 commit into
NousResearch:mainfrom
kyoji2:fix/desktop-mutex-double-quote
Closed

fix(desktop): do not double-quote updateMutex path in withRemoteUpdateMutex#102907
kyoji2 wants to merge 1 commit into
NousResearch:mainfrom
kyoji2:fix/desktop-mutex-double-quote

Conversation

@kyoji2

@kyoji2 kyoji2 commented Sep 4, 2026

Copy link
Copy Markdown

What does this PR do?

Fixes an issue where withRemoteUpdateMutex() double-quoted updateMutex, causing Python to interpret the path as a relative path with literal leading and trailing single quotes. This created a malformed directory named ' under the user's home directory (e.g. $HOME/'/Users/.../.hermes/...).

In addition, two such accidental directory artifacts previously tracked in apps/desktop/ (from running tests locally before this fix) have been removed.

Root Cause

buildSpawnCommand passes updateMutex (produced by expandRemotePath(), which is already shell-escaped as "$HOME"'/.hermes/...') into withRemoteUpdateMutex(command, updateMutex). In withRemoteUpdateMutex, mutexPath was wrapped in shq(mutexPath) again when composing python3 -c .... As a result, Python received sys.argv[1] enclosed in literal single quotes and treated it as a relative path rather than an absolute path.

Related Issue

Fixes #

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 🔒 Security fix
  • 📝 Documentation update
  • ✅ Tests (adding or improving test coverage)
  • ♻️ Refactor (no behavior change)
  • 🎯 New skill (bundled or hub)

Changes Made

  • apps/desktop/electron/remote-lifecycle.ts: Removed redundant shq() wrapper around mutexPath in withRemoteUpdateMutex.
  • apps/desktop/electron/remote-lifecycle.test.ts: Added regression test assertion verifying that updateMutex passed to python3 -c remains expandable without being re-quoted.
  • apps/desktop/'/var/folders/...: Removed two accidentally committed artifact files created by local test runs before this fix.

How to Test

  1. Run the electron lifecycle tests in apps/desktop: 
    npx vitest run electron/remote-lifecycle.test.ts
  2. Verify all tests pass and no directory named ' is created in $HOME or the working directory.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS 15.7.9 (arm64)

Documentation & Housekeeping

  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A

@kyoji2
fix(desktop): do not double-quote updateMutex path in withRemoteUpdat…
e2c35df 
…eMutex

withRemoteUpdateMutex wrapped mutexPath in shq() before passing it
to the inline Python script. Because updateMutex is already formatted
by expandRemotePath() (which yields "$HOME"'/.hermes/...' or single-quoted
paths), wrapping it in shq() again causes Python sys.argv[1] to receive
literal quote characters.

Python's os.makedirs and os.open then interpret the path as a relative
path starting with ', resulting in a malformed directory named ' under
the current working directory (e.g. $HOME/'/Users/.../.mutex'). In fact,
two such paths were previously committed by accident into apps/desktop/.

Remove the redundant shq() wrapper on mutexPath, add a regression test
assertion preventing double-quoting, and remove the tracked accidental
artifact files.
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/desktop Electron desktop app (apps/desktop/*) backend/ssh SSH remote execution area/install-update Installer, updater, packaging, wheels, doctor sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades duplicate This issue or pull request already exists labels Sep 4, 2026
@alt-glitch

alt-glitch commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

This was generated by AI during triage.

Duplicate of #96187 (earliest open PR with the same one-line fix; #99189 is a second competing copy). Same mechanism: withRemoteUpdateMutex re-wraps the already-quoted expandRemotePath() fragment in shq().

One thing unique here worth keeping: this PR also deletes the two stray apps/desktop/'/var/folders/... artifact files that are currently tracked on main — neither #96187 nor #99189 removes them. Maintainers may want that cleanup as its own tiny PR.

@kyoji2

kyoji2 commented Sep 4, 2026

Copy link
Copy Markdown
Author

Closing in favor of #96187 (earliest open fix for the same double-quoting bug; #99189 is the other copy) — no need for a third duplicate in review queue.

The one part unique to this PR — deleting the two stray tracked apps/desktop/'/var/folders/... mutex artifacts — will come back as its own tiny cleanup PR, as suggested in triage.

@kyoji2 kyoji2 closed this Sep 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

area/install-update Installer, updater, packaging, wheels, doctor backend/ssh SSH remote execution comp/desktop Electron desktop app (apps/desktop/*) duplicate This issue or pull request already exists P2 Medium — degraded but workaround exists sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades type/bug Something isn't working

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@kyoji2 @alt-glitch