From 32e0f69abc3c4496c67c4ba7a3a0c8dae70be568 Mon Sep 17 00:00:00 2001 From: Roland Date: Thu, 24 Sep 2026 13:49:15 -0600 Subject: [PATCH] ai: only list custom GGUFs llama-server can load; reset context on custom select Review found two gaps in the custom-model listing. Any *.gguf basename was listed and accepted, but start_llama_server.sh reads active_model back out of ai.yaml through read_config_value.sh. PyYAML dumps non-ASCII as escapes the reader prints literally, the reader strips " #..." before unquoting, and the start script's traversal guard rejects any ".." run. Those names got a 200 from the API and then "Model file does not exist" at startup. A shared is_loadable_model_name check now filters the catalog loop and rejects the name in manage_settings, so the UI never shows an activation that cannot start. Directories named *.gguf are skipped too. Selecting a custom model after a catalog model kept the previous model's explicit context_size (e.g. 16384 from the 4B) because the adopt branch was skipped when there was no catalog default. The default is now always written on model selection, so a custom model resets to null and the conservative default applies. The hint text says so instead of claiming the default is used unconditionally. Co-Authored-By: Claude Fable 5.1 --- app/src/components/Ai.js | 7 +-- modules/ai/api.py | 14 ++++-- modules/ai/catalog.py | 18 ++++++++ modules/ai/test/test_config_catalog.py | 59 +++++++++++++++++++++++++- 4 files changed, 90 insertions(+), 8 deletions(-) diff --git a/app/src/components/Ai.js b/app/src/components/Ai.js index 01197b1d..0da4c9fd 100644 --- a/app/src/components/Ai.js +++ b/app/src/components/Ai.js @@ -187,9 +187,10 @@ export function ManageAi() { is downloaded. Download a model, select it, then enable the assistant. Monitor download progress on the Downloads page.

You can also copy your own GGUF file into ai/models/; it will appear here as a - custom model the next time this page loads. The model must support tool calling - (Qwen3, Llama 3.x, and Mistral instruct models do). Custom models use the default - context size unless you set context_size in ai.yaml.

+ custom model the next time this page loads. Use a plain ASCII file name without + #, quotes, or ... The model must support tool calling (Qwen3, Llama 3.x, + and Mistral instruct models do). Selecting a custom model resets the context size to the + default; set context_size in ai.yaml to override it.

; } diff --git a/modules/ai/api.py b/modules/ai/api.py index 5bf3c44c..366d9977 100644 --- a/modules/ai/api.py +++ b/modules/ai/api.py @@ -750,7 +750,8 @@ async def manage_catalog(_: Request): # here on every request (no file refresh) so it appears in the Active Model dropdown. catalog_names = {i['name'] for i in models} for name, path in sorted(on_disk.items()): - if name not in catalog_names: + # Skip names start_llama_server.sh could not load (see is_loadable_model_name). + if name not in catalog_names and path.is_file() and catalog.is_loadable_model_name(name): models.append(dict( name=name, tier='custom', @@ -788,11 +789,16 @@ async def manage_settings(_: Request, body: schema.AIManageSettingsRequest): if body.active_model is not None: if body.active_model and not (catalog.get_models_directory() / body.active_model).is_file(): raise ValidationError(f'Model is not downloaded: {body.active_model}') + if body.active_model and not catalog.is_loadable_model_name(body.active_model): + raise ValidationError(f'Model file name cannot be loaded by llama-server; rename it to plain ASCII' + f' without #, quotes, or "..": {body.active_model}') config.active_model = body.active_model # Each model has its own context default (the 4B runs 16k, the small tier 8k); selecting - # a model adopts it unless this request also sets context_size explicitly. - if body.context_size is None and (default := catalog.get_model_default_context(body.active_model)): - config.context_size = default + # a model adopts it unless this request also sets context_size explicitly. A custom model + # has no catalog default, so the previous model's explicit value is cleared rather than + # carried over. + if body.context_size is None: + config.context_size = catalog.get_model_default_context(body.active_model) if body.enabled is not None: config.enabled = body.enabled if body.idle_unload_minutes is not None: diff --git a/modules/ai/catalog.py b/modules/ai/catalog.py index 165df81a..416729b4 100644 --- a/modules/ai/catalog.py +++ b/modules/ai/catalog.py @@ -51,6 +51,24 @@ def get_models_directory() -> Path: return get_media_directory() / 'ai/models' +# Printable ASCII only, minus the characters the shell config reader cannot round-trip. +# start_llama_server.sh reads active_model back out of ai.yaml with read_config_value.sh: PyYAML +# escapes non-ASCII (\xE9), the reader strips " #..." before unquoting, and quotes/backslashes +# survive as literal characters. The start script's traversal guard also rejects any ".." run. +_UNLOADABLE_CHARS = set('#"\'\\/') + + +def is_loadable_model_name(name: str) -> bool: + """True when llama-server can be started with this file name from ai/models. + + Catalog names always pass; this exists for GGUFs the user copies in by hand.""" + if not name or not name.endswith('.gguf') or name != name.strip(): + return False + if '..' in name or any(c in _UNLOADABLE_CHARS for c in name): + return False + return all(0x20 <= ord(c) < 0x7F for c in name) + + async def fetch_models_manifest(url: str = None) -> dict: """Fetch the models manifest from the CDN and verify its GPG signature (map-manifest pattern).""" url = url or MODELS_MANIFEST_URL diff --git a/modules/ai/test/test_config_catalog.py b/modules/ai/test/test_config_catalog.py index 91ab97ad..28369004 100644 --- a/modules/ai/test/test_config_catalog.py +++ b/modules/ai/test/test_config_catalog.py @@ -218,13 +218,70 @@ async def test_manage_catalog_lists_custom_models(async_client, test_directory, assert custom['size'] == 40 assert custom['url'] is None - # It can be activated like any downloaded model; it has no catalog context default. + # Selecting a catalog model first stores its context default in ai.yaml ... + content = dict(active_model='Qwen3-1.7B-Q4_K_M.gguf') + request, response = await async_client.post('/api/ai/manage/settings', content=json.dumps(content)) + assert response.status_code == HTTPStatus.OK + assert response.json['context_size'] == 8_192 + + # ... and switching to the custom model clears it, since a custom model has no catalog + # default; it must not silently keep the previous model's value. content = dict(active_model='My-Custom-Model-Q8_0.gguf') request, response = await async_client.post('/api/ai/manage/settings', content=json.dumps(content)) assert response.status_code == HTTPStatus.OK assert response.json['active_model'] == 'My-Custom-Model-Q8_0.gguf' assert response.json['context_size'] is None + assert catalog.get_effective_context_size() == 8_192 # the conservative default + + # An explicit context_size in the same request still wins for a custom model. + content = dict(active_model='My-Custom-Model-Q8_0.gguf', context_size=4_096) + request, response = await async_client.post('/api/ai/manage/settings', content=json.dumps(content)) + assert response.json['context_size'] == 4_096 with mock.patch('modules.ai.catalog.fetch_models_manifest', side_effect=RuntimeError('offline')): request, response = await async_client.get('/api/ai/manage/catalog') custom = next(i for i in response.json['models'] if i['name'] == 'My-Custom-Model-Q8_0.gguf') assert custom['active'] is True + + +@pytest.mark.parametrize('name,loadable', [ + ('My-Custom-Model-Q8_0.gguf', True), + ('model (v2) [q4].gguf', True), # shell-safe once quoted by PyYAML + ('café.gguf', False), # PyYAML dumps \xE9; read_config_value.sh prints the escape literally + ('模型-Q4.gguf', False), + ('model #1.gguf', False), # the reader strips " #..." before unquoting + ('foo..bar.gguf', False), # start_llama_server.sh's traversal guard rejects any ".." + ('it\'s.gguf', False), + ('say "hi".gguf', False), + ('back\\slash.gguf', False), + (' padded.gguf', False), + ('model.bin', False), +]) +def test_is_loadable_model_name(name, loadable): + """Only names start_llama_server.sh can read back out of ai.yaml are loadable.""" + assert catalog.is_loadable_model_name(name) is loadable + + +@pytest.mark.asyncio +async def test_manage_catalog_skips_unloadable_custom_models(async_client, test_directory, test_ai_config): + """A copied-in GGUF whose name cannot round-trip through ai.yaml and the shell reader is neither + listed nor accepted, so the UI never reports an activation that llama-server would refuse.""" + models_dir = test_directory / 'ai/models' + models_dir.mkdir(parents=True) + (models_dir / 'café.gguf').write_bytes(b'GGUF') + (models_dir / 'model #1.gguf').write_bytes(b'GGUF') + (models_dir / 'foo..bar.gguf').write_bytes(b'GGUF') + (models_dir / 'Fine-Model.gguf').write_bytes(b'GGUF') + (models_dir / 'a-directory.gguf').mkdir() + + with mock.patch('modules.ai.catalog.fetch_models_manifest', side_effect=RuntimeError('offline')): + request, response = await async_client.get('/api/ai/manage/catalog') + names = [i['name'] for i in response.json['models']] + assert 'Fine-Model.gguf' in names + for bad in ('café.gguf', 'model #1.gguf', 'foo..bar.gguf', 'a-directory.gguf'): + assert bad not in names + + for bad in ('café.gguf', 'model #1.gguf', 'foo..bar.gguf'): + request, response = await async_client.post('/api/ai/manage/settings', + content=json.dumps(dict(active_model=bad))) + assert response.status_code == HTTPStatus.BAD_REQUEST, bad + assert 'cannot be loaded' in response.json['error']