fix(qmd): reject unreliable npm listings and restore non-mapping models verbatim
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
7f89ae70bd
commit
953401a431
4 files changed
+27
-6
No files matched your search
+5
-3
@@ -74,12 +74,14 @@ class Setup:
|
||||
|
||||
def package_installed(self):
|
||||
# npm list exits non-zero on tree problems (ELSPROBLEMS) but still prints valid JSON.
|
||||
output = self.run(['npm', 'list', '--global', '--depth=0', '--json'], check=False) or '{}'
|
||||
output = self.run(['npm', 'list', '--global', '--depth=0', '--json'], check=False)
|
||||
try:
|
||||
listing = json.loads(output)
|
||||
except json.JSONDecodeError:
|
||||
raise ValueError('npm list did not return JSON.')
|
||||
return PACKAGE in (listing.get('dependencies') or {})
|
||||
if not isinstance(listing, dict) or not isinstance(listing.get('dependencies'), dict):
|
||||
raise ValueError('npm list did not report the global packages.')
|
||||
return PACKAGE in listing['dependencies']
|
||||
|
||||
def require_node(self):
|
||||
if not node_version_ok(self.run(['node', '--version'])):
|
||||
@@ -135,7 +137,7 @@ class Setup:
|
||||
previous = json.loads(output.strip().splitlines()[-1])['previous']
|
||||
except (IndexError, KeyError, TypeError, json.JSONDecodeError):
|
||||
raise ValueError('qmd-config set-models returned no previous models.')
|
||||
state['previous_models'] = previous if isinstance(previous, dict) else None
|
||||
state['previous_models'] = previous
|
||||
state['models_block'] = True
|
||||
atomic_json(self.state_path, state)
|
||||
before = {p.name for p in self.models.iterdir()} if self.models.is_dir() else set()
|
||||
|
||||
@@ -78,6 +78,8 @@ test('runQmdCli runs the CLI entry of QMD_PACKAGE_DIR without a shell', () => {
|
||||
|
||||
test('previousModels captures a user models block, null when absent', () => {
|
||||
assert.deepEqual(previousModels('models:\n embed: mine\ncollections: {}\n', YAML), { embed: 'mine' });
|
||||
assert.equal(previousModels('models: custom\n', YAML), 'custom');
|
||||
assert.deepEqual(previousModels('models: [a, b]\n', YAML), ['a', 'b']);
|
||||
assert.equal(previousModels('collections: {}\n', YAML), null);
|
||||
assert.equal(previousModels('', YAML), null);
|
||||
});
|
||||
|
||||
@@ -13,9 +13,9 @@ spec.loader.exec_module(qmd)
|
||||
|
||||
|
||||
class FakeRunner:
|
||||
def __init__(self, installed=False, gpu='cuda', gpu_result=None, cpu_result=None, previous=None, fail_pull=False):
|
||||
def __init__(self, installed=False, gpu='cuda', gpu_result=None, cpu_result=None, previous=None, fail_pull=False, list_output=None):
|
||||
self.calls, self.installed, self.gpu = [], installed, gpu
|
||||
self.previous, self.fail_pull = previous, fail_pull
|
||||
self.previous, self.fail_pull, self.list_output = previous, fail_pull, list_output
|
||||
self.results = {'gpu': gpu_result, 'cpu': cpu_result}
|
||||
|
||||
def __call__(self, arguments, env=None, timeout=None, check=True):
|
||||
@@ -27,6 +27,8 @@ class FakeRunner:
|
||||
return json.dumps({'previous': self.previous}) + chr(10)
|
||||
if self.fail_pull and arguments[:2] == ['qmd', 'pull']:
|
||||
raise ValueError('pull failed')
|
||||
if 'npm list' in joined and self.list_output is not None:
|
||||
return self.list_output
|
||||
if 'npm list' in joined:
|
||||
return json.dumps({'dependencies': {'@tobilu/qmd': {}} if self.installed else {}})
|
||||
if 'npm install' in joined:
|
||||
@@ -198,6 +200,21 @@ class SetupTest(unittest.TestCase):
|
||||
self.assertTrue(self.setup(FakeRunner(installed=True)).package_installed())
|
||||
self.assertFalse(self.setup(FakeRunner()).package_installed())
|
||||
|
||||
def test_package_installed_rejects_unreliable_listings(self):
|
||||
for output in ('', ' ', 'not json', '{"error": {"code": "ELSPROBLEMS"}}', '{}', '[]'):
|
||||
with self.assertRaises(ValueError, msg=repr(output)):
|
||||
self.setup(FakeRunner(list_output=output)).package_installed()
|
||||
self.assertTrue(self.setup(FakeRunner(list_output='{"dependencies": {"@tobilu/qmd": {}}, "error": {}}')).package_installed())
|
||||
self.assertFalse(self.setup(FakeRunner(list_output='{"dependencies": {}}')).package_installed())
|
||||
|
||||
def test_non_mapping_previous_models_are_restored_verbatim(self):
|
||||
runner = FakeRunner(previous='custom.gguf')
|
||||
setup = self.setup(runner)
|
||||
setup.enable(['claude'])
|
||||
self.assertEqual(self.state()['previous_models'], 'custom.gguf')
|
||||
setup.disable(['claude'])
|
||||
self.assertTrue(any(c[-2:] == ['restore-models', '"custom.gguf"'] for c in runner.calls))
|
||||
|
||||
def test_dry_run_changes_nothing(self):
|
||||
runner = FakeRunner()
|
||||
self.setup(runner, dry_run=True).enable(['claude'])
|
||||
|
||||
Reference in new issue
Block a user