From eeb437e2344f2cc1ffd702363915e2195184284c Mon Sep 17 00:00:00 2001 From: Julian lechner Date: Thu, 24 Sep 2026 16:36:50 +0200 Subject: [PATCH] fix(extensions): preserve existing global packages --- TODO.md | 2 +- scripts/lib/managed-extensions.py | 28 +++++++++++++++++++----- scripts/tests/test-managed-extensions.py | 2 ++ 3 files changed, 26 insertions(+), 6 deletions(-) diff --git a/TODO.md b/TODO.md index b9c5cc4..4069ec6 100644 --- a/TODO.md +++ b/TODO.md @@ -6,7 +6,7 @@ - [x] Keep an installed semantic retrieval runtime when an `auto` benchmark says no. `benchmark()` deletes the whole runtime and ownership state even when `state['clients']` still contains active clients. (`scripts/lib/semantic-retrieval.py:117`, `scripts/lib/semantic-retrieval.py:142`) - [x] Check semantic retrieval ownership and links before copying or deleting its directory. An existing owned runtime's source files are overwritten without comparing recorded hashes; a replaced target symlink is followed on enable, and disabling the last client removes foreign files inside the runtime. (`scripts/lib/semantic-retrieval.py:82`, `scripts/lib/semantic-retrieval.py:100`) - [x] Preserve pre-existing MCPorter server entries. `ensure_mcporter()` adds a server and claims ownership without checking existing configuration; if the add succeeds for an existing name, a later removal deletes that user's entry. (`scripts/lib/managed-extensions.py:395`, `scripts/lib/managed-extensions.py:263`) -- [ ] Preserve pre-existing global CLI packages. `ensure_cli()` and `ensure_qmd()` install and claim npm packages without checking whether they were already installed; later removal can uninstall a user's package. (`scripts/lib/managed-extensions.py:342`, `scripts/lib/managed-extensions.py:378`) +- [x] Preserve pre-existing global CLI packages. `ensure_cli()` and `ensure_qmd()` install and claim npm packages without checking whether they were already installed; later removal can uninstall a user's package. (`scripts/lib/managed-extensions.py:342`, `scripts/lib/managed-extensions.py:378`) - [x] Fetch skill archives by an immutable commit and record that same commit. The archive uses GitHub `HEAD`, while `resolvedRevision` comes from a separate later request, so the recorded revision may not identify the installed payload. (`scripts/lib/managed-extensions.py:94`, `scripts/lib/managed-extensions.py:140`, `scripts/lib/managed-extensions.py:409`) - [x] Protect the semantic retrieval daemon's IPC protocol. It uses a hardcoded authentication key and `multiprocessing.connection.recv()`, which deserializes messages; a process that can reach its socket or pipe can submit arbitrary roots and potentially crafted objects. Use a private per-install credential and a non-pickle wire format, or restrict the endpoint to the current user. (`shared/retrieval/server.py:47`, `shared/retrieval/server.py:306`, `shared/retrieval/server.py:314`) - [x] Write semantic retrieval ownership state atomically. `save_state()` replaces the JSON with a direct write, unlike the extension and selection ledgers; interruption can leave it unreadable and block later reconciliation. (`scripts/lib/semantic-retrieval.py:63`) diff --git a/scripts/lib/managed-extensions.py b/scripts/lib/managed-extensions.py index 5ebc818..1a12a55 100644 --- a/scripts/lib/managed-extensions.py +++ b/scripts/lib/managed-extensions.py @@ -163,6 +163,7 @@ class Manager: self.state = json.loads(self.path.read_text(encoding='utf-8')) self.validate() self.cache = {} + self.new_packages = set() def validate(self): if not isinstance(self.state, dict) or self.state.get('version') != 1 or not isinstance(self.state.get('resources'), list): @@ -212,6 +213,14 @@ class Manager: self.cache[client] = installed_plugins(client, self.home) return self.cache[client] + def global_packages(self): + if 'global_packages' not in self.cache: + result = json.loads(run_command(['npm', 'list', '--global', '--depth=0', '--json'], True, self.home)) + if not isinstance(result, dict) or not isinstance(result.get('dependencies'), dict): + raise ValueError('Invalid global npm package inventory.') + self.cache['global_packages'] = set(result['dependencies']) + return self.cache['global_packages'] + def command(self, arguments): if self.dry_run: if not self.summary: @@ -347,10 +356,16 @@ class Manager: if not re.fullmatch(r'[a-z0-9@/_.-]+', package): raise ValueError('Invalid CLI package.') record = next((r for r in self.state['resources'] if r['client'] == client and r['name'] == entry['name']), None) - installed = shutil.which(package) is not None + installed = package in self.global_packages() + if record is None and installed: + if not self.summary: + print(f'PASS Preserving pre-existing global package: {package}') + return if record is not None and installed and not self.update: return self.command(['npm', 'update' if record is not None and installed else 'install', '--global', package]) + self.global_packages().add(package) + self.new_packages.add(package) if not self.dry_run and record is None: self.state['resources'].append(dict(client=client, name=entry['name'], kind='cli', package=package)) self.save() @@ -383,15 +398,18 @@ class Manager: package = entry['codex_source'] if package != '@tobilu/qmd': raise ValueError('Invalid QMD package.') - if not getattr(self, 'qmd_ready', False): + record = next((r for r in self.state['resources'] if r['client'] == 'codex' and r['name'] == entry['name']), None) + pre_existing = record is None and package in self.global_packages() and package not in self.new_packages + if not getattr(self, 'qmd_ready', False) and not pre_existing and package not in self.global_packages(): self.command(['npm', 'install', '--global', package]) - self.qmd_ready = True + self.global_packages().add(package) + self.new_packages.add(package) + self.qmd_ready = True if client == 'claude': entry = dict(entry, codex_method='plugin') self.ensure(client, entry) return - record = next((r for r in self.state['resources'] if r['client'] == client and r['name'] == entry['name']), None) - if record is not None: + if record is not None or pre_existing: return self.state['resources'].append(dict(client='codex', name=entry['name'], kind='qmd', package=package)) self.save() diff --git a/scripts/tests/test-managed-extensions.py b/scripts/tests/test-managed-extensions.py index 2194a8d..428c2bd 100644 --- a/scripts/tests/test-managed-extensions.py +++ b/scripts/tests/test-managed-extensions.py @@ -53,6 +53,8 @@ class ManagedExtensionTests(unittest.TestCase): self.addCleanup(self.output.__exit__, None, None, None) def command(self, args, summary, home=None): + if args[:2] == ['npm', 'list']: + return '{"dependencies":{}}' self.commands.append(args) if len(args) > 3 and args[2] in ('add', 'install') and args[1] == 'plugin': self.installed[args[0]][args[3]] = {'scope': 'user'}