mirror of
https://github.com/HKUDS/nanobot.git
synced 2026-04-02 09:22:36 +00:00
Merge PR #2204: fix(cron): scope cron state to each workspace with safe default-only migration
fix(cron): scope cron state to each workspace with safe default-only migration
This commit is contained in:
commit
b5c95b1a34
@ -34,7 +34,7 @@ from rich.text import Text
|
||||
|
||||
from nanobot import __logo__, __version__
|
||||
from nanobot.cli.stream import StreamRenderer, ThinkingSpinner
|
||||
from nanobot.config.paths import get_workspace_path
|
||||
from nanobot.config.paths import get_workspace_path, is_default_workspace
|
||||
from nanobot.config.schema import Config
|
||||
from nanobot.utils.helpers import sync_workspace_templates
|
||||
|
||||
@ -479,6 +479,17 @@ def _warn_deprecated_config_keys(config_path: Path | None) -> None:
|
||||
)
|
||||
|
||||
|
||||
def _migrate_cron_store(config: "Config") -> None:
|
||||
"""One-time migration: move legacy global cron store into the workspace."""
|
||||
from nanobot.config.paths import get_cron_dir
|
||||
|
||||
legacy_path = get_cron_dir() / "jobs.json"
|
||||
new_path = config.workspace_path / "cron" / "jobs.json"
|
||||
if legacy_path.is_file() and not new_path.exists():
|
||||
new_path.parent.mkdir(parents=True, exist_ok=True)
|
||||
import shutil
|
||||
shutil.move(str(legacy_path), str(new_path))
|
||||
|
||||
|
||||
# ============================================================================
|
||||
# Gateway / Server
|
||||
@ -496,7 +507,6 @@ def gateway(
|
||||
from nanobot.agent.loop import AgentLoop
|
||||
from nanobot.bus.queue import MessageBus
|
||||
from nanobot.channels.manager import ChannelManager
|
||||
from nanobot.config.paths import get_cron_dir
|
||||
from nanobot.cron.service import CronService
|
||||
from nanobot.cron.types import CronJob
|
||||
from nanobot.heartbeat.service import HeartbeatService
|
||||
@ -515,8 +525,12 @@ def gateway(
|
||||
provider = _make_provider(config)
|
||||
session_manager = SessionManager(config.workspace_path)
|
||||
|
||||
# Create cron service first (callback set after agent creation)
|
||||
cron_store_path = get_cron_dir() / "jobs.json"
|
||||
# Preserve existing single-workspace installs, but keep custom workspaces clean.
|
||||
if is_default_workspace(config.workspace_path):
|
||||
_migrate_cron_store(config)
|
||||
|
||||
# Create cron service with workspace-scoped store
|
||||
cron_store_path = config.workspace_path / "cron" / "jobs.json"
|
||||
cron = CronService(cron_store_path)
|
||||
|
||||
# Create agent with cron service
|
||||
@ -703,7 +717,6 @@ def agent(
|
||||
|
||||
from nanobot.agent.loop import AgentLoop
|
||||
from nanobot.bus.queue import MessageBus
|
||||
from nanobot.config.paths import get_cron_dir
|
||||
from nanobot.cron.service import CronService
|
||||
|
||||
config = _load_runtime_config(config, workspace)
|
||||
@ -712,8 +725,12 @@ def agent(
|
||||
bus = MessageBus()
|
||||
provider = _make_provider(config)
|
||||
|
||||
# Create cron service for tool usage (no callback needed for CLI unless running)
|
||||
cron_store_path = get_cron_dir() / "jobs.json"
|
||||
# Preserve existing single-workspace installs, but keep custom workspaces clean.
|
||||
if is_default_workspace(config.workspace_path):
|
||||
_migrate_cron_store(config)
|
||||
|
||||
# Create cron service with workspace-scoped store
|
||||
cron_store_path = config.workspace_path / "cron" / "jobs.json"
|
||||
cron = CronService(cron_store_path)
|
||||
|
||||
if logs:
|
||||
|
||||
@ -7,6 +7,7 @@ from nanobot.config.paths import (
|
||||
get_cron_dir,
|
||||
get_data_dir,
|
||||
get_legacy_sessions_dir,
|
||||
is_default_workspace,
|
||||
get_logs_dir,
|
||||
get_media_dir,
|
||||
get_runtime_subdir,
|
||||
@ -24,6 +25,7 @@ __all__ = [
|
||||
"get_cron_dir",
|
||||
"get_logs_dir",
|
||||
"get_workspace_path",
|
||||
"is_default_workspace",
|
||||
"get_cli_history_path",
|
||||
"get_bridge_install_dir",
|
||||
"get_legacy_sessions_dir",
|
||||
|
||||
@ -40,6 +40,13 @@ def get_workspace_path(workspace: str | None = None) -> Path:
|
||||
return ensure_dir(path)
|
||||
|
||||
|
||||
def is_default_workspace(workspace: str | Path | None) -> bool:
|
||||
"""Return whether a workspace resolves to nanobot's default workspace path."""
|
||||
current = Path(workspace).expanduser() if workspace is not None else Path.home() / ".nanobot" / "workspace"
|
||||
default = Path.home() / ".nanobot" / "workspace"
|
||||
return current.resolve(strict=False) == default.resolve(strict=False)
|
||||
|
||||
|
||||
def get_cli_history_path() -> Path:
|
||||
"""Return the shared CLI history file path."""
|
||||
return Path.home() / ".nanobot" / "history" / "cli_history"
|
||||
|
||||
@ -333,10 +333,8 @@ def mock_agent_runtime(tmp_path):
|
||||
"""Mock agent command dependencies for focused CLI tests."""
|
||||
config = Config()
|
||||
config.agents.defaults.workspace = str(tmp_path / "default-workspace")
|
||||
cron_dir = tmp_path / "data" / "cron"
|
||||
|
||||
with patch("nanobot.config.loader.load_config", return_value=config) as mock_load_config, \
|
||||
patch("nanobot.config.paths.get_cron_dir", return_value=cron_dir), \
|
||||
patch("nanobot.cli.commands.sync_workspace_templates") as mock_sync_templates, \
|
||||
patch("nanobot.cli.commands._make_provider", return_value=object()), \
|
||||
patch("nanobot.cli.commands._print_agent_response") as mock_print_response, \
|
||||
@ -413,7 +411,6 @@ def test_agent_config_sets_active_path(monkeypatch, tmp_path: Path) -> None:
|
||||
lambda path: seen.__setitem__("config_path", path),
|
||||
)
|
||||
monkeypatch.setattr("nanobot.config.loader.load_config", lambda _path=None: config)
|
||||
monkeypatch.setattr("nanobot.config.paths.get_cron_dir", lambda: config_file.parent / "cron")
|
||||
monkeypatch.setattr("nanobot.cli.commands.sync_workspace_templates", lambda _path: None)
|
||||
monkeypatch.setattr("nanobot.cli.commands._make_provider", lambda _config: object())
|
||||
monkeypatch.setattr("nanobot.bus.queue.MessageBus", lambda: object())
|
||||
@ -438,6 +435,147 @@ def test_agent_config_sets_active_path(monkeypatch, tmp_path: Path) -> None:
|
||||
assert seen["config_path"] == config_file.resolve()
|
||||
|
||||
|
||||
def test_agent_uses_workspace_directory_for_cron_store(monkeypatch, tmp_path: Path) -> None:
|
||||
config_file = tmp_path / "instance" / "config.json"
|
||||
config_file.parent.mkdir(parents=True)
|
||||
config_file.write_text("{}")
|
||||
|
||||
config = Config()
|
||||
config.agents.defaults.workspace = str(tmp_path / "agent-workspace")
|
||||
seen: dict[str, Path] = {}
|
||||
|
||||
monkeypatch.setattr("nanobot.config.loader.set_config_path", lambda _path: None)
|
||||
monkeypatch.setattr("nanobot.config.loader.load_config", lambda _path=None: config)
|
||||
monkeypatch.setattr("nanobot.cli.commands.sync_workspace_templates", lambda _path: None)
|
||||
monkeypatch.setattr("nanobot.cli.commands._make_provider", lambda _config: object())
|
||||
monkeypatch.setattr("nanobot.bus.queue.MessageBus", lambda: object())
|
||||
|
||||
class _FakeCron:
|
||||
def __init__(self, store_path: Path) -> None:
|
||||
seen["cron_store"] = store_path
|
||||
|
||||
class _FakeAgentLoop:
|
||||
def __init__(self, *args, **kwargs) -> None:
|
||||
pass
|
||||
|
||||
async def process_direct(self, *_args, **_kwargs):
|
||||
return OutboundMessage(channel="cli", chat_id="direct", content="ok")
|
||||
|
||||
async def close_mcp(self) -> None:
|
||||
return None
|
||||
|
||||
monkeypatch.setattr("nanobot.cron.service.CronService", _FakeCron)
|
||||
monkeypatch.setattr("nanobot.agent.loop.AgentLoop", _FakeAgentLoop)
|
||||
monkeypatch.setattr("nanobot.cli.commands._print_agent_response", lambda *_args, **_kwargs: None)
|
||||
|
||||
result = runner.invoke(app, ["agent", "-m", "hello", "-c", str(config_file)])
|
||||
|
||||
assert result.exit_code == 0
|
||||
assert seen["cron_store"] == config.workspace_path / "cron" / "jobs.json"
|
||||
|
||||
|
||||
def test_agent_workspace_override_does_not_migrate_legacy_cron(
|
||||
monkeypatch, tmp_path: Path
|
||||
) -> None:
|
||||
config_file = tmp_path / "instance" / "config.json"
|
||||
config_file.parent.mkdir(parents=True)
|
||||
config_file.write_text("{}")
|
||||
|
||||
legacy_dir = tmp_path / "global" / "cron"
|
||||
legacy_dir.mkdir(parents=True)
|
||||
legacy_file = legacy_dir / "jobs.json"
|
||||
legacy_file.write_text('{"jobs": []}')
|
||||
|
||||
override = tmp_path / "override-workspace"
|
||||
config = Config()
|
||||
seen: dict[str, Path] = {}
|
||||
|
||||
monkeypatch.setattr("nanobot.config.loader.set_config_path", lambda _path: None)
|
||||
monkeypatch.setattr("nanobot.config.loader.load_config", lambda _path=None: config)
|
||||
monkeypatch.setattr("nanobot.cli.commands.sync_workspace_templates", lambda _path: None)
|
||||
monkeypatch.setattr("nanobot.cli.commands._make_provider", lambda _config: object())
|
||||
monkeypatch.setattr("nanobot.bus.queue.MessageBus", lambda: object())
|
||||
monkeypatch.setattr("nanobot.config.paths.get_cron_dir", lambda: legacy_dir)
|
||||
|
||||
class _FakeCron:
|
||||
def __init__(self, store_path: Path) -> None:
|
||||
seen["cron_store"] = store_path
|
||||
|
||||
class _FakeAgentLoop:
|
||||
def __init__(self, *args, **kwargs) -> None:
|
||||
pass
|
||||
|
||||
async def process_direct(self, *_args, **_kwargs):
|
||||
return OutboundMessage(channel="cli", chat_id="direct", content="ok")
|
||||
|
||||
async def close_mcp(self) -> None:
|
||||
return None
|
||||
|
||||
monkeypatch.setattr("nanobot.cron.service.CronService", _FakeCron)
|
||||
monkeypatch.setattr("nanobot.agent.loop.AgentLoop", _FakeAgentLoop)
|
||||
monkeypatch.setattr("nanobot.cli.commands._print_agent_response", lambda *_args, **_kwargs: None)
|
||||
|
||||
result = runner.invoke(
|
||||
app,
|
||||
["agent", "-m", "hello", "-c", str(config_file), "-w", str(override)],
|
||||
)
|
||||
|
||||
assert result.exit_code == 0
|
||||
assert seen["cron_store"] == override / "cron" / "jobs.json"
|
||||
assert legacy_file.exists()
|
||||
assert not (override / "cron" / "jobs.json").exists()
|
||||
|
||||
|
||||
def test_agent_custom_config_workspace_does_not_migrate_legacy_cron(
|
||||
monkeypatch, tmp_path: Path
|
||||
) -> None:
|
||||
config_file = tmp_path / "instance" / "config.json"
|
||||
config_file.parent.mkdir(parents=True)
|
||||
config_file.write_text("{}")
|
||||
|
||||
legacy_dir = tmp_path / "global" / "cron"
|
||||
legacy_dir.mkdir(parents=True)
|
||||
legacy_file = legacy_dir / "jobs.json"
|
||||
legacy_file.write_text('{"jobs": []}')
|
||||
|
||||
custom_workspace = tmp_path / "custom-workspace"
|
||||
config = Config()
|
||||
config.agents.defaults.workspace = str(custom_workspace)
|
||||
seen: dict[str, Path] = {}
|
||||
|
||||
monkeypatch.setattr("nanobot.config.loader.set_config_path", lambda _path: None)
|
||||
monkeypatch.setattr("nanobot.config.loader.load_config", lambda _path=None: config)
|
||||
monkeypatch.setattr("nanobot.cli.commands.sync_workspace_templates", lambda _path: None)
|
||||
monkeypatch.setattr("nanobot.cli.commands._make_provider", lambda _config: object())
|
||||
monkeypatch.setattr("nanobot.bus.queue.MessageBus", lambda: object())
|
||||
monkeypatch.setattr("nanobot.config.paths.get_cron_dir", lambda: legacy_dir)
|
||||
|
||||
class _FakeCron:
|
||||
def __init__(self, store_path: Path) -> None:
|
||||
seen["cron_store"] = store_path
|
||||
|
||||
class _FakeAgentLoop:
|
||||
def __init__(self, *args, **kwargs) -> None:
|
||||
pass
|
||||
|
||||
async def process_direct(self, *_args, **_kwargs):
|
||||
return OutboundMessage(channel="cli", chat_id="direct", content="ok")
|
||||
|
||||
async def close_mcp(self) -> None:
|
||||
return None
|
||||
|
||||
monkeypatch.setattr("nanobot.cron.service.CronService", _FakeCron)
|
||||
monkeypatch.setattr("nanobot.agent.loop.AgentLoop", _FakeAgentLoop)
|
||||
monkeypatch.setattr("nanobot.cli.commands._print_agent_response", lambda *_args, **_kwargs: None)
|
||||
|
||||
result = runner.invoke(app, ["agent", "-m", "hello", "-c", str(config_file)])
|
||||
|
||||
assert result.exit_code == 0
|
||||
assert seen["cron_store"] == custom_workspace / "cron" / "jobs.json"
|
||||
assert legacy_file.exists()
|
||||
assert not (custom_workspace / "cron" / "jobs.json").exists()
|
||||
|
||||
|
||||
def test_agent_overrides_workspace_path(mock_agent_runtime):
|
||||
workspace_path = Path("/tmp/agent-workspace")
|
||||
|
||||
@ -544,7 +682,7 @@ def test_gateway_workspace_option_overrides_config(monkeypatch, tmp_path: Path)
|
||||
assert config.workspace_path == override
|
||||
|
||||
|
||||
def test_gateway_uses_config_directory_for_cron_store(monkeypatch, tmp_path: Path) -> None:
|
||||
def test_gateway_uses_workspace_directory_for_cron_store(monkeypatch, tmp_path: Path) -> None:
|
||||
config_file = tmp_path / "instance" / "config.json"
|
||||
config_file.parent.mkdir(parents=True)
|
||||
config_file.write_text("{}")
|
||||
@ -555,7 +693,6 @@ def test_gateway_uses_config_directory_for_cron_store(monkeypatch, tmp_path: Pat
|
||||
|
||||
monkeypatch.setattr("nanobot.config.loader.set_config_path", lambda _path: None)
|
||||
monkeypatch.setattr("nanobot.config.loader.load_config", lambda _path=None: config)
|
||||
monkeypatch.setattr("nanobot.config.paths.get_cron_dir", lambda: config_file.parent / "cron")
|
||||
monkeypatch.setattr("nanobot.cli.commands.sync_workspace_templates", lambda _path: None)
|
||||
monkeypatch.setattr("nanobot.cli.commands._make_provider", lambda _config: object())
|
||||
monkeypatch.setattr("nanobot.bus.queue.MessageBus", lambda: object())
|
||||
@ -571,7 +708,130 @@ def test_gateway_uses_config_directory_for_cron_store(monkeypatch, tmp_path: Pat
|
||||
result = runner.invoke(app, ["gateway", "--config", str(config_file)])
|
||||
|
||||
assert isinstance(result.exception, _StopGatewayError)
|
||||
assert seen["cron_store"] == config_file.parent / "cron" / "jobs.json"
|
||||
assert seen["cron_store"] == config.workspace_path / "cron" / "jobs.json"
|
||||
|
||||
|
||||
def test_gateway_workspace_override_does_not_migrate_legacy_cron(
|
||||
monkeypatch, tmp_path: Path
|
||||
) -> None:
|
||||
config_file = tmp_path / "instance" / "config.json"
|
||||
config_file.parent.mkdir(parents=True)
|
||||
config_file.write_text("{}")
|
||||
|
||||
legacy_dir = tmp_path / "global" / "cron"
|
||||
legacy_dir.mkdir(parents=True)
|
||||
legacy_file = legacy_dir / "jobs.json"
|
||||
legacy_file.write_text('{"jobs": []}')
|
||||
|
||||
override = tmp_path / "override-workspace"
|
||||
config = Config()
|
||||
seen: dict[str, Path] = {}
|
||||
|
||||
monkeypatch.setattr("nanobot.config.loader.set_config_path", lambda _path: None)
|
||||
monkeypatch.setattr("nanobot.config.loader.load_config", lambda _path=None: config)
|
||||
monkeypatch.setattr("nanobot.cli.commands.sync_workspace_templates", lambda _path: None)
|
||||
monkeypatch.setattr("nanobot.cli.commands._make_provider", lambda _config: object())
|
||||
monkeypatch.setattr("nanobot.bus.queue.MessageBus", lambda: object())
|
||||
monkeypatch.setattr("nanobot.session.manager.SessionManager", lambda _workspace: object())
|
||||
monkeypatch.setattr("nanobot.config.paths.get_cron_dir", lambda: legacy_dir)
|
||||
|
||||
class _StopCron:
|
||||
def __init__(self, store_path: Path) -> None:
|
||||
seen["cron_store"] = store_path
|
||||
raise _StopGatewayError("stop")
|
||||
|
||||
monkeypatch.setattr("nanobot.cron.service.CronService", _StopCron)
|
||||
|
||||
result = runner.invoke(
|
||||
app,
|
||||
["gateway", "--config", str(config_file), "--workspace", str(override)],
|
||||
)
|
||||
|
||||
assert isinstance(result.exception, _StopGatewayError)
|
||||
assert seen["cron_store"] == override / "cron" / "jobs.json"
|
||||
assert legacy_file.exists()
|
||||
assert not (override / "cron" / "jobs.json").exists()
|
||||
|
||||
|
||||
def test_gateway_custom_config_workspace_does_not_migrate_legacy_cron(
|
||||
monkeypatch, tmp_path: Path
|
||||
) -> None:
|
||||
config_file = tmp_path / "instance" / "config.json"
|
||||
config_file.parent.mkdir(parents=True)
|
||||
config_file.write_text("{}")
|
||||
|
||||
legacy_dir = tmp_path / "global" / "cron"
|
||||
legacy_dir.mkdir(parents=True)
|
||||
legacy_file = legacy_dir / "jobs.json"
|
||||
legacy_file.write_text('{"jobs": []}')
|
||||
|
||||
custom_workspace = tmp_path / "custom-workspace"
|
||||
config = Config()
|
||||
config.agents.defaults.workspace = str(custom_workspace)
|
||||
seen: dict[str, Path] = {}
|
||||
|
||||
monkeypatch.setattr("nanobot.config.loader.set_config_path", lambda _path: None)
|
||||
monkeypatch.setattr("nanobot.config.loader.load_config", lambda _path=None: config)
|
||||
monkeypatch.setattr("nanobot.cli.commands.sync_workspace_templates", lambda _path: None)
|
||||
monkeypatch.setattr("nanobot.cli.commands._make_provider", lambda _config: object())
|
||||
monkeypatch.setattr("nanobot.bus.queue.MessageBus", lambda: object())
|
||||
monkeypatch.setattr("nanobot.session.manager.SessionManager", lambda _workspace: object())
|
||||
monkeypatch.setattr("nanobot.config.paths.get_cron_dir", lambda: legacy_dir)
|
||||
|
||||
class _StopCron:
|
||||
def __init__(self, store_path: Path) -> None:
|
||||
seen["cron_store"] = store_path
|
||||
raise _StopGatewayError("stop")
|
||||
|
||||
monkeypatch.setattr("nanobot.cron.service.CronService", _StopCron)
|
||||
|
||||
result = runner.invoke(app, ["gateway", "--config", str(config_file)])
|
||||
|
||||
assert isinstance(result.exception, _StopGatewayError)
|
||||
assert seen["cron_store"] == custom_workspace / "cron" / "jobs.json"
|
||||
assert legacy_file.exists()
|
||||
assert not (custom_workspace / "cron" / "jobs.json").exists()
|
||||
|
||||
|
||||
def test_migrate_cron_store_moves_legacy_file(tmp_path: Path) -> None:
|
||||
"""Legacy global jobs.json is moved into the workspace on first run."""
|
||||
from nanobot.cli.commands import _migrate_cron_store
|
||||
|
||||
legacy_dir = tmp_path / "global" / "cron"
|
||||
legacy_dir.mkdir(parents=True)
|
||||
legacy_file = legacy_dir / "jobs.json"
|
||||
legacy_file.write_text('{"jobs": []}')
|
||||
|
||||
config = Config()
|
||||
config.agents.defaults.workspace = str(tmp_path / "workspace")
|
||||
workspace_cron = config.workspace_path / "cron" / "jobs.json"
|
||||
|
||||
with patch("nanobot.config.paths.get_cron_dir", return_value=legacy_dir):
|
||||
_migrate_cron_store(config)
|
||||
|
||||
assert workspace_cron.exists()
|
||||
assert workspace_cron.read_text() == '{"jobs": []}'
|
||||
assert not legacy_file.exists()
|
||||
|
||||
|
||||
def test_migrate_cron_store_skips_when_workspace_file_exists(tmp_path: Path) -> None:
|
||||
"""Migration does not overwrite an existing workspace cron store."""
|
||||
from nanobot.cli.commands import _migrate_cron_store
|
||||
|
||||
legacy_dir = tmp_path / "global" / "cron"
|
||||
legacy_dir.mkdir(parents=True)
|
||||
(legacy_dir / "jobs.json").write_text('{"old": true}')
|
||||
|
||||
config = Config()
|
||||
config.agents.defaults.workspace = str(tmp_path / "workspace")
|
||||
workspace_cron = config.workspace_path / "cron" / "jobs.json"
|
||||
workspace_cron.parent.mkdir(parents=True)
|
||||
workspace_cron.write_text('{"new": true}')
|
||||
|
||||
with patch("nanobot.config.paths.get_cron_dir", return_value=legacy_dir):
|
||||
_migrate_cron_store(config)
|
||||
|
||||
assert workspace_cron.read_text() == '{"new": true}'
|
||||
|
||||
|
||||
def test_gateway_uses_configured_port_when_cli_flag_is_missing(monkeypatch, tmp_path: Path) -> None:
|
||||
|
||||
@ -10,6 +10,7 @@ from nanobot.config.paths import (
|
||||
get_media_dir,
|
||||
get_runtime_subdir,
|
||||
get_workspace_path,
|
||||
is_default_workspace,
|
||||
)
|
||||
|
||||
|
||||
@ -40,3 +41,9 @@ def test_shared_and_legacy_paths_remain_global() -> None:
|
||||
def test_workspace_path_is_explicitly_resolved() -> None:
|
||||
assert get_workspace_path() == Path.home() / ".nanobot" / "workspace"
|
||||
assert get_workspace_path("~/custom-workspace") == Path.home() / "custom-workspace"
|
||||
|
||||
|
||||
def test_is_default_workspace_distinguishes_default_and_custom_paths() -> None:
|
||||
assert is_default_workspace(None) is True
|
||||
assert is_default_workspace(Path.home() / ".nanobot" / "workspace") is True
|
||||
assert is_default_workspace("~/custom-workspace") is False
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user