Skip to content

chore(desktop): drop the committed update-mutex artifact directory - #102191

Closed
jonpol01 wants to merge 1 commit into
NousResearch:mainfrom
jonpol01:chore/remove-committed-update-mutex-artifact
Closed

chore(desktop): drop the committed update-mutex artifact directory#102191
jonpol01 wants to merge 1 commit into
NousResearch:mainfrom
jonpol01:chore/remove-committed-update-mutex-artifact

Conversation

@jonpol01

@jonpol01 jonpol01 commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Summary

apps/desktop/' is a tracked directory whose name is a literal single quote. Inside it, a full absolute path is materialised as nested subdirectories:

apps/desktop/'/var/folders/5h/.../T/hermes-update-mutex-LMF9y5/home/.hermes-update-in-progress.mutex'
apps/desktop/'/var/folders/5h/.../T/hermes-update-mutex-us8HZu/home/.hermes-update-in-progress.mutex'

Both files are 0 bytes, nothing in the tree references them, and the leading and trailing ' are part of the filenames — they are shell quotes that were never stripped.

They arrived in 36620578f0, an unrelated menu-label commit, via a git add ..

Where they come from

withRemoteUpdateMutex() builds its Python invocation as:

return `python3 -c ${shq(script)} ${shq(mutexPath)} ${shq(command)}`

mutexPath comes from expandRemotePath(), which already returns a shell-quoted string (it has to — it is interpolated bare into shell commands like [ ! -e ${marker} ], and the ~ form returns "$HOME"'/…' so the shell still expands $HOME). Quoting it a second time means Python receives the quotes as part of the filename:

$ python3 -c 'import sys;print(repr(sys.argv[1]))' ''\''/opt/data/.hermes-update-in-progress.mutex'\'''
"'/opt/data/.hermes-update-in-progress.mutex'"

A path beginning with ' is relative, so the script's os.makedirs(os.path.dirname(mutex_path)) recreates the entire absolute path underneath whatever directory the process was started in. Running apps/desktop/electron/remote-lifecycle.test.ts from apps/desktop reproduces the directory immediately — that is how these two files came to exist.

Scope

This PR only deletes the committed artifact. The quoting fault that generates it is a separate concern and is already covered upstream:

I deliberately did not touch remote-lifecycle.ts here — #99189 already makes exactly the one-line change (${shq(mutexPath)}${mutexPath}) plus a matching assertion, and a fourth copy of that fix would not help anyone. But none of those PRs deletes these two files, so the litter survives whichever of them lands. That is the gap this closes.

Test plan

  • git ls-tree -r origin/main --name-only | grep "^apps/desktop/'" → exactly the 2 files, both 0 bytes (git cat-file -s → 0).
  • Nothing references them: no hit for hermes-update-mutex-LMF9y5, hermes-update-mutex-us8HZu, or the temp-dir hash anywhere in the tree. (git grep "apps/desktop/'" matches only @apps/desktop/ string literals in the composer path tests — unrelated.)
  • Not covered by an existing ignore rule (git check-ignore reports nothing), so they were tracked deliberately-looking but accidentally.
  • apps/desktop/electron/remote-lifecycle.test.ts: 91 passed, before and after.
  • Removal is content-only: 2 files changed, 0 insertions, 0 deletions.

`apps/desktop/'` is not a real path. It is a literal single-quote directory
holding a full absolute path as nested subdirectories:

    apps/desktop/'/var/folders/5h/.../hermes-update-mutex-LMF9y5/home/.hermes-update-in-progress.mutex'

Both files are 0 bytes, nothing in the tree references them, and the leading
and trailing `'` are part of the filenames. They are the fingerprint of the
double-quoted mutex path in `withRemoteUpdateMutex()`: the path reaches Python
with its shell quotes still attached, so it is treated as CWD-relative and
`os.makedirs()` materialises the whole absolute path under whatever directory
the process happened to be in. Running
`apps/desktop/electron/remote-lifecycle.test.ts` from `apps/desktop`
reproduces it on the spot.

They were swept in by a `git add .` in 3662057, an unrelated menu-label
commit.

This only removes the committed artifact. The generator is a separate concern
already covered by open PRs (NousResearch#99189, NousResearch#96187, NousResearch#96260) and issues (NousResearch#99133,
NousResearch#96212, NousResearch#96188); those repair the quoting but none of them deletes these two
files, so the litter would survive whichever one lands.
@alt-glitch alt-glitch added type/refactor Code restructuring, no behavior change P3 Low — cosmetic, nice to have comp/desktop Electron desktop app (apps/desktop/*) labels Sep 3, 2026
@Enough1122

Copy link
Copy Markdown
Contributor

AI code review — automated review for reference; please use your judgment.

Chunk5 review — chore(desktop): drop committed update-mutex artifacts

Deletes two empty committed files whose paths embed a macOS /var/folders/... temp dir — clearly accidental artifacts of an update-mutex test run, not source.

  • Both files are 0-byte (e69de29... empty blob); deletion is zero-risk.
  • Non-blocking: worth a follow-up .gitignore entry for hermes-update-mutex-* / .hermes-update-in-progress.mutex (and checking how a /var/folders temp path ended up committed under apps/desktop/) so the next test run doesn't re-commit them.

Non-blocking. Merge.

@kshitijk4poor

Copy link
Copy Markdown
Collaborator

Salvaged into #104206 — your commit 4c8e2d165f is cherry-picked there with authorship preserved (author John Paul Soliva), alongside the fix for what created those files in the first place: withRemoteUpdateMutex double-quoted the mutex path, so main's own detached backend does not inherit the update mutex descriptor test ran python with a quote-prefixed relative path and os.makedirs built apps/desktop/'/var/... in the checkout. With the fix in the same PR the directory cannot regenerate (confirmed: the test run leaves no ' dir on the branch; on origin/main it recreates it).

Re the automated review on this PR: the deletion is correct and complete (both zero-byte files, no consumers — the only hermes-update-mutex- hit in apps/desktop is the mkdtemp prefix in the test). Auto-merge is armed on #104206.

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

Labels

comp/desktop Electron desktop app (apps/desktop/*) P3 Low — cosmetic, nice to have type/refactor Code restructuring, no behavior change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants