From 7ebd61f35b746491adc3e0136b46ad218dadab78 Mon Sep 17 00:00:00 2001 From: Julian lechner Date: Thu, 8 Oct 2026 12:41:19 +0200 Subject: [PATCH] fix(qmd): make removal wait for the daemon and survive partial failures Removal reads the daemon PID before --stop and waits up to 10 s for it to exit. A model file that cannot be deleted stays recorded in the state for a retry instead of aborting removal, and a failing models block restore only warns. Co-Authored-By: Claude Opus 5.5 --- scripts/lib/qmd.py | 57 ++++++++++++++++++++++++++---- scripts/tests/test-qmd-setup.py | 61 +++++++++++++++++++++++++++++++++ 2 files changed, 111 insertions(+), 7 deletions(-) diff --git a/scripts/lib/qmd.py b/scripts/lib/qmd.py index 2501ee5..3f0b3e2 100644 --- a/scripts/lib/qmd.py +++ b/scripts/lib/qmd.py @@ -9,6 +9,7 @@ import shutil import subprocess import sys import tempfile +import time PACKAGE = '@tobilu/qmd' VERSION = '2.8.3' @@ -25,6 +26,27 @@ def run(arguments, env=None, timeout=1800, check=True): return result.stdout +def process_alive(pid): + if WINDOWS: # os.kill(pid, 0) would send CTRL_C_EVENT on Windows + import ctypes + kernel32 = ctypes.windll.kernel32 + handle = kernel32.OpenProcess(0x1000, False, pid) # PROCESS_QUERY_LIMITED_INFORMATION + if not handle: + return False + code = ctypes.c_ulong() + try: + return bool(kernel32.GetExitCodeProcess(handle, ctypes.byref(code))) and code.value == 259 # STILL_ACTIVE + finally: + kernel32.CloseHandle(handle) + try: + os.kill(pid, 0) + except ProcessLookupError: + return False + except PermissionError: + return True + return True + + def decide(results): for device in ('gpu', 'cpu'): if any(r.get('device') == device and r.get('passed') is True for r in results): @@ -182,11 +204,20 @@ class Setup: self.say(f'QMD local search enabled on {state["device"].upper()}.') def remove_all(self, state): + daemon = self.scripts / 'daemon.json' + try: + pid = json.loads(daemon.read_text(encoding='utf-8')).get('pid') if daemon.is_file() else None + except (OSError, ValueError, AttributeError): + pid = None try: if self.scripts.joinpath('qmd-search.mjs').exists(): self.node('qmd-search.mjs', '--stop', timeout=60) - except (ValueError, subprocess.SubprocessError): + except (ValueError, OSError, subprocess.SubprocessError): pass + if isinstance(pid, int) and pid > 0: + deadline = time.monotonic() + 10 # the daemon must release the model files and the package + while process_alive(pid) and time.monotonic() < deadline: + time.sleep(0.2) collections = self.scripts / 'collections.json' names = json.loads(collections.read_text(encoding='utf-8')) if collections.exists() else [] for name in names: @@ -197,19 +228,31 @@ class Setup: pass if state['models_block'] and self.scripts.joinpath('qmd-config.mjs').exists(): previous = state.get('previous_models') - if previous is not None: - self.node('qmd-config.mjs', 'restore-models', json.dumps(previous)) - else: - self.node('qmd-config.mjs', 'unset-models') + try: + if previous is not None: + self.node('qmd-config.mjs', 'restore-models', json.dumps(previous)) + else: + self.node('qmd-config.mjs', 'unset-models') + except (ValueError, OSError, subprocess.SubprocessError) as error: + print(f'QMD local search: could not restore the models block in index.yml: {error}', file=sys.stderr) + kept = [] for name in state['owned_models']: path = self.models / name if name == Path(name).name and path.is_file() and not path.is_symlink(): - path.unlink() + try: + path.unlink() + except OSError as error: + print(f'QMD local search: could not delete model {name}; the next removal retries it: {error}', file=sys.stderr) + kept.append(name) if state['owned_package']: self.run(['npm', 'uninstall', '--global', PACKAGE]) if self.scripts.exists() and not self.scripts.is_symlink(): shutil.rmtree(self.scripts) - self.state_path.unlink(missing_ok=True) + if kept: + state.update(clients=[], owned_models=kept, owned_package=False, models_block=False, previous_models=None, files={}) + atomic_json(self.state_path, state) + else: + self.state_path.unlink(missing_ok=True) def disable(self, clients): state = self.load() diff --git a/scripts/tests/test-qmd-setup.py b/scripts/tests/test-qmd-setup.py index cbfbac4..4aa16df 100644 --- a/scripts/tests/test-qmd-setup.py +++ b/scripts/tests/test-qmd-setup.py @@ -290,6 +290,67 @@ class SetupTest(unittest.TestCase): self.setup(FakeRunner(installed=True, fail=('server.py',))).enable(['claude']) self.assertFalse(legacy.exists()) + def test_removal_waits_for_the_daemon_to_exit_before_deleting_models(self): + runner = FakeRunner() + setup = self.setup(runner) + setup.enable(['claude']) + (self.scripts_dir() / 'daemon.json').write_text(json.dumps({'port': 1, 'token': 't', 'pid': 4242})) + checks = [] + + def alive(pid): + checks.append((pid, any('--stop' in c for c in runner.calls), any('npm uninstall' in ' '.join(c) for c in runner.calls))) + return len(checks) < 3 + with mock.patch.object(qmd, 'process_alive', alive): + setup.disable(['claude']) + self.assertEqual(checks, [(4242, True, False)] * 3, 'the PID is polled after --stop and before the uninstall') + self.assertIn(['npm', 'uninstall', '--global', '@tobilu/qmd'], runner.calls) + + def test_process_alive(self): + self.assertTrue(qmd.process_alive(os.getpid())) + child = subprocess.Popen([sys.executable, '-c', 'pass']) + child.wait() + self.assertFalse(qmd.process_alive(child.pid)) + + def test_undeletable_model_keeps_state_for_a_retry(self): + runner = FakeRunner() + setup = self.setup(runner) + setup.enable(['claude']) + state = self.state() + models = self.home / '.cache/qmd/models' + (models / 'locked.gguf').write_text('x') + (models / 'free.gguf').write_text('x') + state['owned_models'] = ['free.gguf', 'locked.gguf'] + (self.home / '.my-ai-configuration/qmd.json').write_text(json.dumps(state)) + real = Path.unlink + + def unlink(path, missing_ok=False): + if path.name == 'locked.gguf': + raise PermissionError('in use') + real(path, missing_ok=missing_ok) + with mock.patch.object(qmd.Path, 'unlink', unlink): + setup.disable(['claude']) + state = self.state() + self.assertEqual(state['owned_models'], ['locked.gguf']) + self.assertFalse(state['owned_package']) + self.assertFalse(state['models_block']) + self.assertFalse((models / 'free.gguf').exists()) + self.assertIn(['npm', 'uninstall', '--global', '@tobilu/qmd'], runner.calls) + self.assertFalse(self.scripts_dir().exists()) + runner.calls.clear() + setup.disable(['claude']) + self.assertFalse((models / 'locked.gguf').exists()) + self.assertFalse((self.home / '.my-ai-configuration/qmd.json').exists()) + self.assertFalse(any('npm uninstall' in ' '.join(c) for c in runner.calls), 'a retry never uninstalls a later foreign package') + + def test_failing_models_block_restore_does_not_stop_removal(self): + runner = FakeRunner(fail=('unset-models',)) + setup = self.setup(runner) + setup.enable(['claude']) + setup.disable(['claude']) + self.assertIn(['npm', 'uninstall', '--global', '@tobilu/qmd'], runner.calls) + self.assertFalse(self.scripts_dir().exists()) + self.assertFalse((self.home / '.my-ai-configuration/qmd.json').exists()) + if __name__ == '__main__': unittest.main()