mirror of
https://github.com/HKUDS/nanobot.git
synced 2026-08-04 08:28:36 +00:00
fix(config): warn on legacy model migration
This commit is contained in:
parent
8bd53d6e26
commit
4e7c57eb1a
@ -1348,7 +1348,7 @@ Contributor notes for adding new providers live in [`development.md`](./developm
|
|||||||
|
|
||||||
Model presets let you name a complete model configuration and select one per session with `/model <preset>`. Configure all model, provider, generation, context-window, and image-input settings under top-level `modelPresets`; `agents.defaults` only selects preset names.
|
Model presets let you name a complete model configuration and select one per session with `/model <preset>`. Configure all model, provider, generation, context-window, and image-input settings under top-level `modelPresets`; `agents.defaults` only selects preset names.
|
||||||
|
|
||||||
On first load, nanobot migrates legacy model fields from `agents.defaults` and inline fallback objects in `config.json` into named presets, then atomically rewrites the file. If a concrete `modelPresets.default` and legacy direct fields both exist, the concrete preset wins and the legacy fields are removed. Legacy model fields supplied through nested `NANOBOT_AGENTS` environment settings are not supported.
|
On first load, nanobot migrates legacy model fields from `agents.defaults` and inline fallback objects in `config.json` into named presets, then atomically rewrites the file and logs a warning. If a concrete `modelPresets.default` and legacy direct fields both exist, the concrete preset wins and the warning explains that the conflicting legacy fields were removed. Legacy model fields supplied through nested `NANOBOT_AGENTS` environment settings are not supported and produce a warning with instructions to move them into `modelPresets`.
|
||||||
|
|
||||||
```json
|
```json
|
||||||
{
|
{
|
||||||
|
|||||||
@ -6,6 +6,7 @@ import re
|
|||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
from typing import Any
|
from typing import Any
|
||||||
|
|
||||||
|
from loguru import logger
|
||||||
from pydantic import BaseModel, ValidationError
|
from pydantic import BaseModel, ValidationError
|
||||||
from pydantic_settings import SettingsError
|
from pydantic_settings import SettingsError
|
||||||
|
|
||||||
@ -16,6 +17,7 @@ from nanobot.utils.helpers import _write_text_atomic
|
|||||||
# Global variable to store current config path (for multi-instance support)
|
# Global variable to store current config path (for multi-instance support)
|
||||||
_current_config_path: Path | None = None
|
_current_config_path: Path | None = None
|
||||||
_schema_refs_ready = False
|
_schema_refs_ready = False
|
||||||
|
_warned_legacy_model_env = False
|
||||||
|
|
||||||
|
|
||||||
def set_config_path(path: Path) -> None:
|
def set_config_path(path: Path) -> None:
|
||||||
@ -67,6 +69,7 @@ def load_config(config_path: Path | None = None) -> Config:
|
|||||||
summary="Environment-based configuration is invalid.",
|
summary="Environment-based configuration is invalid.",
|
||||||
issues=validation_issues(exc),
|
issues=validation_issues(exc),
|
||||||
) from exc
|
) from exc
|
||||||
|
_warn_unsupported_legacy_model_env(path)
|
||||||
_apply_ssrf_whitelist(config)
|
_apply_ssrf_whitelist(config)
|
||||||
return config
|
return config
|
||||||
|
|
||||||
@ -110,6 +113,7 @@ def load_config(config_path: Path | None = None) -> Config:
|
|||||||
),
|
),
|
||||||
)
|
)
|
||||||
|
|
||||||
|
legacy_model_migration = _legacy_model_migration_kind(data)
|
||||||
data, migrated = _migrate_config(data)
|
data, migrated = _migrate_config(data)
|
||||||
try:
|
try:
|
||||||
config = Config.model_validate(data)
|
config = Config.model_validate(data)
|
||||||
@ -124,7 +128,21 @@ def load_config(config_path: Path | None = None) -> Config:
|
|||||||
|
|
||||||
if migrated:
|
if migrated:
|
||||||
_write_text_atomic(path, json.dumps(data, indent=2, ensure_ascii=False))
|
_write_text_atomic(path, json.dumps(data, indent=2, ensure_ascii=False))
|
||||||
|
if legacy_model_migration:
|
||||||
|
detail = (
|
||||||
|
"Existing modelPresets.default took precedence; conflicting "
|
||||||
|
"legacy agents.defaults fields were removed."
|
||||||
|
if legacy_model_migration == "conflict"
|
||||||
|
else "Legacy settings were converted to named model presets."
|
||||||
|
)
|
||||||
|
logger.warning(
|
||||||
|
"Migrated legacy model configuration in {}. {} "
|
||||||
|
"Review the rewritten file before downgrading nanobot.",
|
||||||
|
path,
|
||||||
|
detail,
|
||||||
|
)
|
||||||
|
|
||||||
|
_warn_unsupported_legacy_model_env(path)
|
||||||
_apply_ssrf_whitelist(config)
|
_apply_ssrf_whitelist(config)
|
||||||
return config
|
return config
|
||||||
|
|
||||||
@ -332,6 +350,69 @@ _LEGACY_MODEL_FIELD_ALIASES = {
|
|||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
def _legacy_model_migration_kind(data: dict[str, Any]) -> str | None:
|
||||||
|
"""Classify a pending model migration without exposing configured values."""
|
||||||
|
if not _needs_legacy_model_migration(data):
|
||||||
|
return None
|
||||||
|
|
||||||
|
agents = data.get("agents")
|
||||||
|
defaults = agents.get("defaults") if isinstance(agents, dict) else None
|
||||||
|
presets = data.get("modelPresets", data.get("model_presets"))
|
||||||
|
has_legacy_fields = isinstance(defaults, dict) and any(
|
||||||
|
alias in defaults
|
||||||
|
for aliases in _LEGACY_MODEL_FIELD_ALIASES.values()
|
||||||
|
for alias in aliases
|
||||||
|
)
|
||||||
|
if has_legacy_fields and isinstance(presets, dict) and "default" in presets:
|
||||||
|
return "conflict"
|
||||||
|
return "migrated"
|
||||||
|
|
||||||
|
|
||||||
|
def _has_unsupported_legacy_model_env() -> bool:
|
||||||
|
for env_name in ("NANOBOT_AGENTS", "NANOBOT_AGENTS__DEFAULTS"):
|
||||||
|
raw = os.environ.get(env_name)
|
||||||
|
if not raw:
|
||||||
|
continue
|
||||||
|
try:
|
||||||
|
parsed = json.loads(raw)
|
||||||
|
except json.JSONDecodeError:
|
||||||
|
continue
|
||||||
|
data = (
|
||||||
|
{"agents": parsed}
|
||||||
|
if env_name == "NANOBOT_AGENTS"
|
||||||
|
else {"agents": {"defaults": parsed}}
|
||||||
|
)
|
||||||
|
if isinstance(parsed, dict) and _needs_legacy_model_migration(data):
|
||||||
|
return True
|
||||||
|
|
||||||
|
legacy_suffixes = {
|
||||||
|
alias.upper()
|
||||||
|
for aliases in _LEGACY_MODEL_FIELD_ALIASES.values()
|
||||||
|
for alias in aliases
|
||||||
|
}
|
||||||
|
prefix = "NANOBOT_AGENTS__DEFAULTS__"
|
||||||
|
for env_name in os.environ:
|
||||||
|
upper_name = env_name.upper()
|
||||||
|
if not upper_name.startswith(prefix):
|
||||||
|
continue
|
||||||
|
suffix = upper_name[len(prefix):]
|
||||||
|
if suffix in legacy_suffixes:
|
||||||
|
return True
|
||||||
|
return False
|
||||||
|
|
||||||
|
|
||||||
|
def _warn_unsupported_legacy_model_env(config_path: Path) -> None:
|
||||||
|
global _warned_legacy_model_env
|
||||||
|
if _warned_legacy_model_env or not _has_unsupported_legacy_model_env():
|
||||||
|
return
|
||||||
|
logger.warning(
|
||||||
|
"Ignoring unsupported legacy model settings from NANOBOT_AGENTS. "
|
||||||
|
"Move them to modelPresets in {}.",
|
||||||
|
config_path,
|
||||||
|
)
|
||||||
|
_warned_legacy_model_env = True
|
||||||
|
|
||||||
|
|
||||||
def _pop_alias(mapping: dict[str, Any], aliases: tuple[str, ...]) -> tuple[bool, Any]:
|
def _pop_alias(mapping: dict[str, Any], aliases: tuple[str, ...]) -> tuple[bool, Any]:
|
||||||
found = False
|
found = False
|
||||||
value: Any = None
|
value: Any = None
|
||||||
|
|||||||
@ -3,11 +3,26 @@ import socket
|
|||||||
from unittest.mock import patch
|
from unittest.mock import patch
|
||||||
|
|
||||||
import pytest
|
import pytest
|
||||||
|
from loguru import logger
|
||||||
|
|
||||||
from nanobot.config.loader import load_config, save_config
|
from nanobot.config.loader import load_config, save_config
|
||||||
from nanobot.security.network import validate_url_target
|
from nanobot.security.network import validate_url_target
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.fixture
|
||||||
|
def warning_messages():
|
||||||
|
messages: list[str] = []
|
||||||
|
sink_id = logger.add(
|
||||||
|
lambda message: messages.append(str(message)),
|
||||||
|
level="WARNING",
|
||||||
|
format="{message}",
|
||||||
|
)
|
||||||
|
try:
|
||||||
|
yield messages
|
||||||
|
finally:
|
||||||
|
logger.remove(sink_id)
|
||||||
|
|
||||||
|
|
||||||
def _fake_resolve(host: str, results: list[str]):
|
def _fake_resolve(host: str, results: list[str]):
|
||||||
"""Return a getaddrinfo mock that maps the given host to fake IP results."""
|
"""Return a getaddrinfo mock that maps the given host to fake IP results."""
|
||||||
def _resolver(hostname, port, family=0, type_=0):
|
def _resolver(hostname, port, family=0, type_=0):
|
||||||
@ -127,7 +142,10 @@ def test_save_config_drops_legacy_max_messages(tmp_path) -> None:
|
|||||||
assert "max_messages" not in saved["agents"]["defaults"]
|
assert "max_messages" not in saved["agents"]["defaults"]
|
||||||
|
|
||||||
|
|
||||||
def test_load_config_rewrites_legacy_model_fields_to_default_preset(tmp_path) -> None:
|
def test_load_config_rewrites_legacy_model_fields_to_default_preset(
|
||||||
|
tmp_path,
|
||||||
|
warning_messages,
|
||||||
|
) -> None:
|
||||||
config_path = tmp_path / "config.json"
|
config_path = tmp_path / "config.json"
|
||||||
config_path.write_text(
|
config_path.write_text(
|
||||||
json.dumps({
|
json.dumps({
|
||||||
@ -151,9 +169,29 @@ def test_load_config_rewrites_legacy_model_fields_to_default_preset(tmp_path) ->
|
|||||||
assert "model" not in saved["agents"]["defaults"]
|
assert "model" not in saved["agents"]["defaults"]
|
||||||
assert saved["modelPresets"]["default"]["model"] == "openai/gpt-4.1"
|
assert saved["modelPresets"]["default"]["model"] == "openai/gpt-4.1"
|
||||||
assert saved["modelPresets"]["default"]["temperature"] == 0
|
assert saved["modelPresets"]["default"]["temperature"] == 0
|
||||||
|
migration_warnings = [
|
||||||
|
message
|
||||||
|
for message in warning_messages
|
||||||
|
if "Migrated legacy model configuration" in message
|
||||||
|
]
|
||||||
|
assert len(migration_warnings) == 1
|
||||||
|
assert "Legacy settings were converted to named model presets" in migration_warnings[0]
|
||||||
|
assert "Review the rewritten file before downgrading" in migration_warnings[0]
|
||||||
|
|
||||||
|
load_config(config_path)
|
||||||
|
|
||||||
|
migration_warnings = [
|
||||||
|
message
|
||||||
|
for message in warning_messages
|
||||||
|
if "Migrated legacy model configuration" in message
|
||||||
|
]
|
||||||
|
assert len(migration_warnings) == 1
|
||||||
|
|
||||||
|
|
||||||
def test_load_config_prefers_existing_default_preset_over_legacy_fields(tmp_path) -> None:
|
def test_load_config_prefers_existing_default_preset_over_legacy_fields(
|
||||||
|
tmp_path,
|
||||||
|
warning_messages,
|
||||||
|
) -> None:
|
||||||
config_path = tmp_path / "config.json"
|
config_path = tmp_path / "config.json"
|
||||||
config_path.write_text(
|
config_path.write_text(
|
||||||
json.dumps({
|
json.dumps({
|
||||||
@ -185,11 +223,17 @@ def test_load_config_prefers_existing_default_preset_over_legacy_fields(tmp_path
|
|||||||
assert "model" not in saved["agents"]["defaults"]
|
assert "model" not in saved["agents"]["defaults"]
|
||||||
assert "provider" not in saved["agents"]["defaults"]
|
assert "provider" not in saved["agents"]["defaults"]
|
||||||
assert "maxTokens" not in saved["agents"]["defaults"]
|
assert "maxTokens" not in saved["agents"]["defaults"]
|
||||||
|
assert any(
|
||||||
|
"Existing modelPresets.default took precedence; conflicting "
|
||||||
|
"legacy agents.defaults fields were removed" in message
|
||||||
|
for message in warning_messages
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
def test_load_config_does_not_migrate_legacy_model_fields_from_environment(
|
def test_load_config_does_not_migrate_legacy_model_fields_from_environment(
|
||||||
tmp_path,
|
tmp_path,
|
||||||
monkeypatch,
|
monkeypatch,
|
||||||
|
warning_messages,
|
||||||
) -> None:
|
) -> None:
|
||||||
monkeypatch.setenv(
|
monkeypatch.setenv(
|
||||||
"NANOBOT_AGENTS",
|
"NANOBOT_AGENTS",
|
||||||
@ -207,9 +251,25 @@ def test_load_config_does_not_migrate_legacy_model_fields_from_environment(
|
|||||||
assert config.resolve_default_preset().model == "anthropic/claude-opus-4-5"
|
assert config.resolve_default_preset().model == "anthropic/claude-opus-4-5"
|
||||||
assert config.resolve_default_preset().provider == "auto"
|
assert config.resolve_default_preset().provider == "auto"
|
||||||
assert config.resolve_default_preset().max_tokens == 8192
|
assert config.resolve_default_preset().max_tokens == 8192
|
||||||
|
assert any(
|
||||||
|
"Ignoring unsupported legacy model settings from NANOBOT_AGENTS" in message
|
||||||
|
for message in warning_messages
|
||||||
|
)
|
||||||
|
|
||||||
|
load_config(tmp_path / "another-missing-config.json")
|
||||||
|
|
||||||
|
environment_warnings = [
|
||||||
|
message
|
||||||
|
for message in warning_messages
|
||||||
|
if "Ignoring unsupported legacy model settings from NANOBOT_AGENTS" in message
|
||||||
|
]
|
||||||
|
assert len(environment_warnings) == 1
|
||||||
|
|
||||||
|
|
||||||
def test_load_config_migrates_inline_fallback_to_named_preset(tmp_path) -> None:
|
def test_load_config_migrates_inline_fallback_to_named_preset(
|
||||||
|
tmp_path,
|
||||||
|
warning_messages,
|
||||||
|
) -> None:
|
||||||
config_path = tmp_path / "config.json"
|
config_path = tmp_path / "config.json"
|
||||||
config_path.write_text(
|
config_path.write_text(
|
||||||
json.dumps({
|
json.dumps({
|
||||||
@ -233,6 +293,10 @@ def test_load_config_migrates_inline_fallback_to_named_preset(tmp_path) -> None:
|
|||||||
assert config.agents.defaults.fallback_models == ["claude-sonnet-4"]
|
assert config.agents.defaults.fallback_models == ["claude-sonnet-4"]
|
||||||
assert saved["agents"]["defaults"]["fallbackModels"] == ["claude-sonnet-4"]
|
assert saved["agents"]["defaults"]["fallbackModels"] == ["claude-sonnet-4"]
|
||||||
assert saved["modelPresets"]["claude-sonnet-4"]["provider"] == "anthropic"
|
assert saved["modelPresets"]["claude-sonnet-4"]["provider"] == "anthropic"
|
||||||
|
assert any(
|
||||||
|
"Legacy settings were converted to named model presets." in message
|
||||||
|
for message in warning_messages
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
def test_onboard_refresh_backfills_missing_channel_fields(tmp_path, monkeypatch) -> None:
|
def test_onboard_refresh_backfills_missing_channel_fields(tmp_path, monkeypatch) -> None:
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user