fix(desktop): allow updater handoff for gateway-only venv locks - #75380
nanami7777777 wants to merge 2 commits into
Conversation
teknium1
left a comment
There was a problem hiding this comment.
Thanks for tracing the updater handoff path; current main does have the reported initial lock-timeout abort at apps/desktop/electron/main.ts:2815-2819.
Problems
- The new success return after the timeout does not reach the updater.
applyUpdatesimmediately runsscanVenvBlockersatapps/desktop/electron/main.ts:2943and aborts a blocked scan at:2945-2952. That scanner deliberately treats a process running fromvenv\\Scripts\\python(w).exeas a holder (hermes_cli/update_cmd.py:2638-2640), which includes the proposed gateway-only case. update-lock-handoff.test.tstests the classifier in isolation but not this subsequent preflight, so the currently unreachable handoff is not covered.
Suggested changes
- Thread one conservative gateway-only classification through both Desktop gates, or make the existing preflight expose a validated classification that permits only the exact gateway-only set. Keep non-gateway and probe-failure paths blocking.
- Add an
applyUpdates-path regression test proving gateway-only reaches updater spawn while aserveholder remains blocked.
Automated hermes-sweeper review.
| @@ -2762,6 +2763,16 @@ async function releaseBackendLock(updateRoot, tag) { | |||
| `[${tag}] venv shim still locked after 15s; aborting hand-off (something outside this app holds the venv)` | |||
| ) | |||
|
|
|||
| const remainingHolders = listWindowsInstallVenvHolders(updateRoot) | |||
There was a problem hiding this comment.
This only bypasses the first Desktop gate. applyUpdates immediately calls scanVenvBlockers on current main (main.ts:2943), and that scanner treats same-install venv\\Scripts\\pythonw.exe gateways as blockers (hermes_cli/update_cmd.py:2638-2640), so the flow still aborts before spawning the updater. Carry the same validated gateway-only classification through that preflight and cover the full sequence in a regression test.
|
Good catch. You were right that my first pass only opened the Desktop-side timeout gate, while the subsequent Windows venv-holder preflight in the Python update path would still stop the handoff. I pushed a follow-up that threads a conservative gateway-only allowance through both stages:
I also added regression coverage on both sides:
Validation I ran locally:
|
14d455e to
aa7c905
Compare
Summary
gateway runprocessesProblem
Issue #75334 describes a real dead-end in the Desktop self-update flow on Windows: the desktop preflight waits for the venv shim lock to disappear, but it only kills backends the desktop itself owns. If the remaining lock holder is a same-install
hermes gateway runprocess, Desktop aborts the update even thoughhermes updatealready knows how to pause and resume Windows gateways safely.Fix
After the 15s Desktop-side unlock wait expires, this change inspects the remaining same-install venv holders. If they are all gateway processes, Desktop now hands off to the updater instead of aborting, letting the existing
hermes updatepause/resume logic finish the job. Non-gateway holders still block the update exactly as before.Testing
cd apps/desktop && npx vitest run --project electron electron/update-lock-handoff.test.ts electron/windows-child-options.test.tscd apps/desktop && npx tsc -p tsconfig.electron.json --noEmitCloses #75334