Fix #2213: Bug: admin/restart is self-destructive on Windows (pkill/bash/tsx unavailable, d - #2214
Conversation
…emTensor#2213) `POST /api/v1/admin/restart` used to run `pkill` + `bash -c "... tsx bridge.cts --daemon"` and then `SIGTERM` its own PID after 200 ms. On Windows the first two calls are silent no-ops (pkill/bash unavailable), but the self-shutdown always succeeds, so the Memory Viewer daemon died with nothing to respawn it — including on every "Save settings" click, since SettingsView triggers the restart overlay after save. Guard the Hermes restart and clear-data flows with a new `isWindowsPlatform()` check + `ServerOptions.platform` seam. When the runtime is Windows and no supervisor (launchd / systemd / NSSM-style INVOCATION_ID) is detected, we now: * skip the pkill+bash+shutdown sequence, * keep the daemon alive, * return `{ ok: false, restarting: false, manualRestartRequired: true, message: ... }` from /admin/restart, and * still wipe the DB / WAL / SHM / bridge-status files from /admin/clear-data but return `restarting: false, manualRestartRequired: true` afterwards. Supervised Windows hosts keep the original self-shutdown path so the supervisor can respawn the process. Viewer `stores/restart.ts` recognises the new payload and enters a new `manualRestartRequired` phase (EN + 中文 copy), and `RestartOverlay` renders it with the same terminal treatment as `restartFailed`.
Memtensor-AI
added
ai:generated
🤖 Open Code ReviewTarget: PR #2214 ✅ OpenCodeReview: No comments generated. Looks good to me. Generated by cloud-assistant via Open Code Review. |
🔧 Open Code Review requested Agent fixOpen Code Review found 8 issue(s). I have resumed the development Agent to fix them.
The Agent will push a new commit to this PR branch. OCR will recheck after the commit is pushed. |
- Return ok:true on the restart route's Windows manual path so the contract matches clear-data; typed SDK wrappers that read `ok` as a success discriminant no longer misclassify the intentional decline. - Report the resolved `platform` variable in both Windows manual responses instead of hardcoding `"win32"`, so tests that inject `lifecycle.platform` see the value they set. - Hoist `agent`/`supervised`/`platform` to the top of the restart handler, mirroring the clear-data handler so a future early-exit never leaves the Windows guard unreachable. - Rewrite the clear-data Windows comment to say the Hermes chat is NOT killed on Windows (the previous wording implied handle cleanup in a branch that skips it). - Move `platform` under `ServerOptions.lifecycle` alongside `supervised` / `requestShutdown`; they are the same category of process/environment override. Update the admin tests to match. - Replace the 4-deep nested ternaries in RestartOverlay's message and hint with plain if/else blocks per the review checklist. - Add a `RestartResponse` interface and use `api.post<RestartResponse>` in `triggerRestart` so the manualRestartRequired branch drops the four ad-hoc casts and gets contract-checked at compile time.
✅ Automated Test Results: PASSEDAll tests passed (91/91 executed). memos_local_plugin/unit: 82/82, memos_python_core/changed-repo-python: 9/9. Duration: 10s Branch: |
Memtensor-AI
added
status:ready
|
Synced this branch with the current
Regression evidence:
The PR is conflict-free and currently |
3 participants
Description
Fixed #2213:
POST /api/v1/admin/restart(and the same code path in/api/v1/admin/clear-data) no longer self-destructs the Memory Viewer daemon on Windows. On Windows portable installs the historicalpkill+bash -c "... tsx bridge.cts --daemon"sequence silently failed while the follow-upSIGTERM-self-kill always succeeded, so the daemon died with no replacement — a fault that also triggered on every "Save settings" click becauseSettingsView.tsxposts to/admin/restartafter each save.The fix adds an
isWindowsPlatform()guard inapps/memos-local-plugin/server/routes/admin.ts(with a newServerOptions.platformseam so tests inject the platform cleanly). When the runtime is Windows AND no supervisor is detected (INVOCATION_ID/XPC_SERVICE_NAMEabsent), we now skip the pkill+bash+shutdown chain, keep the daemon alive, and return{ ok: false, restarting: false, manualRestartRequired: true, message: ... }for/admin/restart;/admin/clear-datastill tears downMemoryCoreand unlinks the DB / WAL / SHM / bridge-status files but returnsrestarting: false, manualRestartRequired: trueand lets the user restart Hermes themselves. Supervised Windows hosts keep the original self-shutdown path so a supervisor can respawn the daemon.Viewer client updates:
viewer/src/stores/restart.ts::triggerRestart()recognises the new payload and enters a newmanualRestartRequiredphase (instead of endlessly polling a healthy server), andRestartOverlay.tsxrenders the phase with the same terminal treatment asrestartFailed. Bilingual copy added underrestart.manualRequired/restart.manualRequiredHintinviewer/src/stores/i18n.ts.Verified:
tests/unit/server/admin.test.ts— 7/7 green (3 new Windows-specific tests + 4 preserved). Broadertests/unit/server/ tests/unit/config/— 137 passing, 1 pre-existing failure inhttp.test.ts::POST /migrate/openclaw/runthat reproduces on the untouched base commit (confirmed via git-stash sandwich). Bothtsc -p tsconfig.jsonandtsc -p tsconfig.viewer.jsoncomplete with no errors. cc @whipser030 @hijzyRelated Issue (Required): Fixes #2213
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Automated tests are pending.
Checklist
@whipser030, @hijzy please review this PR.
Reviewer Checklist