From cd7480945b4d5e6e645c5e1721c00301a2dd9341 Mon Sep 17 00:00:00 2001 From: Xubin Ren <52506698+Re-bin@users.noreply.github.com> Date: Wed, 12 Aug 2026 21:46:56 +0900 Subject: [PATCH] fix(session): preserve history across storage relocation Co-authored-by: lmzopq <1646888+lmzopq@users.noreply.github.com> --- conftest.py | 23 +- docs/architecture.md | 4 +- docs/cli-reference.md | 18 + docs/concepts.md | 10 +- docs/deployment.md | 2 +- docs/guides/deploy-nanobot-gateway.md | 4 +- docs/multiple-instances.md | 3 +- docs/quick-start.md | 2 +- docs/troubleshooting.md | 3 +- nanobot/agent/loop.py | 6 + nanobot/cli/commands.py | 38 ++ nanobot/config/loader.py | 2 + nanobot/config/schema.py | 13 +- nanobot/session/manager.py | 492 ++++++++++++++++++++++-- tests/cli/test_session_restore.py | 31 ++ tests/config/test_config_load_errors.py | 19 +- tests/session/test_session_location.py | 196 +++++++++- tests/test_nanobot_facade.py | 10 +- 18 files changed, 820 insertions(+), 56 deletions(-) create mode 100644 tests/cli/test_session_restore.py diff --git a/conftest.py b/conftest.py index 5457f3796..abf47c532 100644 --- a/conftest.py +++ b/conftest.py @@ -25,14 +25,27 @@ def _isolate_nanobot_log_activation() -> Iterator[None]: @pytest.fixture(autouse=True) def _isolate_sessions_root(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> Iterator[None]: - """Redirect session storage away from the real ~/.nanobot/sessions. + """Redirect session storage away from the real active config data directory. - Session storage lives under get_legacy_sessions_dir() (outside the workspace, + Session storage lives under the active runtime data root (outside the workspace, per ADR-0001), so without redirection tests would write into the real home. """ - root = tmp_path / "sessions-root" - monkeypatch.setattr("nanobot.session.manager.get_legacy_sessions_dir", lambda: root) - monkeypatch.setattr("nanobot.config.paths.get_legacy_sessions_dir", lambda: root) + runtime_root = tmp_path.parent / f"{tmp_path.name}-runtime-root" + legacy_root = tmp_path.parent / f"{tmp_path.name}-legacy-sessions-root" + + def runtime_subdir(name: str) -> Path: + path = runtime_root / name + path.mkdir(parents=True, exist_ok=True) + return path + + monkeypatch.setattr( + "nanobot.session.manager.get_runtime_subdir", + runtime_subdir, + ) + monkeypatch.setattr( + "nanobot.session.manager.get_legacy_sessions_dir", + lambda: legacy_root, + ) yield diff --git a/docs/architecture.md b/docs/architecture.md index 2158ef8be..249471ba0 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -149,7 +149,7 @@ Defaults: |---|---| | Config | `~/.nanobot/config.json` | | Workspace | `~/.nanobot/workspace/` | -| Sessions | `~/.nanobot/sessions//*.jsonl` | +| Sessions | `/sessions//*.jsonl` (default: `~/.nanobot/sessions/...`) | | Memory | `/memory/` | | Cron store | `/cron/jobs.json` | | WebUI/media/log runtime data | config directory subdirectories such as `webui/`, `media/`, and `logs/` | @@ -180,7 +180,7 @@ Session history is the near-term conversation replay. Memory is the longer-term | Store | File area | |---|---| -| Session JSONL files | `~/.nanobot/sessions//` | +| Session JSONL files | `/sessions//` | | Long-term memory | `/memory/MEMORY.md` | | Consolidation source history | `/memory/history.jsonl` | | Bootstrap identity files | `/SOUL.md`, `/USER.md`, templates under `nanobot/templates/` | diff --git a/docs/cli-reference.md b/docs/cli-reference.md index 247170506..26d65d271 100644 --- a/docs/cli-reference.md +++ b/docs/cli-reference.md @@ -94,6 +94,24 @@ follow the printed WebUI **Settings → Models** or `nanobot onboard --wizard` r | `nanobot agent --no-markdown` | Print plain text instead of Rich-rendered Markdown | | `nanobot agent --logs` | Show runtime logs while chatting | +## Session Storage and Rollback + +Session JSONL files live under `/sessions//`, outside the +agent-readable workspace. On the first upgraded start, nanobot safely migrates existing +`/sessions/*.jsonl` files after verifying an atomic copy. Stop every old nanobot +process that uses the workspace before upgrading; old and new binaries must not write the +same session concurrently. + +To prepare a downgrade, stop nanobot and copy the current sessions back to the path understood +by older releases: + +```bash +nanobot sessions restore-workspace --config ./bot-a/config.json --workspace ./bot-a/workspace +``` + +The command never deletes the external store and refuses to overwrite a different existing +workspace file. Back up both the config directory and workspace before changing versions. + In interactive mode, `Enter` sends the current message. Press `Alt+Enter` to add a newline before sending. Interactive mode exits with `exit`, `quit`, `/exit`, `/quit`, `:q`, or `Ctrl+D`. diff --git a/docs/concepts.md b/docs/concepts.md index ac65a9ab4..00dfef726 100644 --- a/docs/concepts.md +++ b/docs/concepts.md @@ -27,7 +27,7 @@ The default instance lives under `~/.nanobot/`: |---|---| | `~/.nanobot/config.json` | Instance configuration: providers, model defaults, channels, tools, gateway, API, and runtime options | | `~/.nanobot/workspace/` | Agent workspace: memory, heartbeat tasks, cron jobs, skills, and generated artifacts | -| `~/.nanobot/sessions//` | Session history stored outside the agent-accessible workspace and namespaced by its canonical path | +| `~/.nanobot/sessions//` | Session history stored outside the agent-accessible workspace; the opaque ID follows workspace moves | You can override both with command flags: @@ -126,11 +126,17 @@ nanobot uses two related stores: | Store | Location | Purpose | |---|---|---| -| Sessions | `~/.nanobot/sessions//*.jsonl` | Recent conversation turns replayed into context | +| Sessions | `/sessions//*.jsonl` | Recent conversation turns replayed into context | | Memory | `/memory/MEMORY.md` and `/memory/history.jsonl` | Long-term facts and consolidated history | Dream is a periodic consolidation job. It reads accumulated history and updates workspace memory so useful context can survive beyond short session replay. +The configured workspace contains a `.nanobot/workspace-id` file. It contains only an +opaque random identifier—never conversation content or credentials. Keep it with workspace +backups: it lets nanobot find the same external session namespace after the workspace is +renamed, moved, or restored. A live copy opened alongside the original receives a new ID so +the two workspaces do not share conversations accidentally. + See [`memory.md`](./memory.md) for the detailed design. ## Apps and Agent Plugins diff --git a/docs/deployment.md b/docs/deployment.md index 63f78c506..506a7e6f9 100644 --- a/docs/deployment.md +++ b/docs/deployment.md @@ -11,7 +11,7 @@ Check these once before Render, Docker, systemd, or LaunchAgent: | `nanobot status` shows the expected config and workspace | Confirms the process will read the instance you meant to run | | `nanobot agent -m "Hello!"` works | Proves install, config, provider, model, and workspace writes before adding a service layer | | Secrets are in environment variables or protected config files | API keys, bot tokens, OAuth state, and chat credentials should not be world-readable | -| `~/.nanobot/` (including `sessions/`) and any custom config/workspace paths are persistent | Sessions, memory, channel login state, generated artifacts, and cron jobs live there | +| The active config directory (including `sessions/`) and workspace are persistent | Sessions follow `--config`; memory, generated artifacts, and the workspace identity marker follow the workspace | | Channel access control is intentional | Use `allowFrom`, pairing, WebSocket `token`/`tokenIssueSecret`, or private test channels before exposing the bot | | Ports are planned | Gateway health defaults to local-only `127.0.0.1:18790`; WebUI/WebSocket defaults to `8765`; `nanobot serve` defaults to `8900` | | Logs are easy to reach | Use `docker compose logs`, `journalctl`, LaunchAgent log files, or `nanobot gateway --verbose` while diagnosing startup | diff --git a/docs/guides/deploy-nanobot-gateway.md b/docs/guides/deploy-nanobot-gateway.md index 3353e9201..2825cfc6d 100644 --- a/docs/guides/deploy-nanobot-gateway.md +++ b/docs/guides/deploy-nanobot-gateway.md @@ -47,8 +47,8 @@ nanobot gateway logs - Docker Compose is the most repeatable Linux container path. - systemd user services are useful for Linux user-level gateway deployments. - macOS LaunchAgent keeps the gateway alive after login. -- Persist `~/.nanobot/sessions/` together with config, workspace, memory files, - channel login state, and generated artifacts. +- Persist the active config directory's `sessions/` folder together with the workspace + (including `.nanobot/workspace-id`), memory files, channel login state, and generated artifacts. - Restart the gateway after editing `config.json`. ## Security notes diff --git a/docs/multiple-instances.md b/docs/multiple-instances.md index fd7a9b8d6..ca7cfa706 100644 --- a/docs/multiple-instances.md +++ b/docs/multiple-instances.md @@ -58,6 +58,7 @@ nanobot agent -c ~/.nanobot-telegram/config.json -w /tmp/nanobot-telegram-test |-----------|---------------|---------| | **Config** | `--config` path | `~/.nanobot-A/config.json` | | **Workspace** | `--workspace` or config | `~/.nanobot-A/workspace/` | +| **Sessions** | config directory + workspace ID | `~/.nanobot-A/sessions//` | | **Cron Jobs** | workspace directory | `~/.nanobot-A/workspace/cron/` | | **Media / runtime state** | config directory | `~/.nanobot-A/media/` | @@ -126,6 +127,6 @@ nanobot gateway --config ~/.nanobot-telegram/config.json --workspace /tmp/nanobo ## Notes - Each instance must use a different port if they run at the same time -- Use a different workspace per instance if you want isolated memory, sessions, and skills +- Session data follows the active config directory; use a different workspace per instance to isolate memory, skills, and the stable session namespace ID - `--workspace` overrides the workspace defined in the config file - Cron jobs are stored in the active workspace; runtime media/state is derived from the config directory diff --git a/docs/quick-start.md b/docs/quick-start.md index adf4a3d62..3877ae546 100644 --- a/docs/quick-start.md +++ b/docs/quick-start.md @@ -49,7 +49,7 @@ The WebUI launcher creates or updates: |---|---| | `~/.nanobot/config.json` | Provider, model, WebUI, channel, tool, and runtime settings | | `~/.nanobot/workspace/` | Memory, skills, automations, and generated files | -| `~/.nanobot/sessions//` | Recent session history, isolated by canonical workspace path | +| `~/.nanobot/sessions//` | Recent session history stored outside the workspace; the ID remains stable across workspace moves | If the installer did not open the browser, run: diff --git a/docs/troubleshooting.md b/docs/troubleshooting.md index 1501af0ce..607963c61 100644 --- a/docs/troubleshooting.md +++ b/docs/troubleshooting.md @@ -319,7 +319,8 @@ See [`chat-apps.md`](./chat-apps.md) for channel-specific setup. |---|---| | Conversation context seems wrong | Confirm the active workspace and session. WebUI chats and chat app threads may use different sessions. | | Memory does not update immediately | Dream consolidation is periodic; recent turns still live in session history. | -| Old sessions appear after moving config | Session files are stored under `~/.nanobot/sessions//`; verify the canonical workspace path recorded in the directory's `.workspace` marker. | +| Sessions disappear after changing `--config` | Sessions follow the config directory at `/sessions//`; use the original config path or copy that `sessions/` directory into the new config directory while nanobot is stopped. | +| Sessions disappear after moving a workspace | Keep the workspace's `.nanobot/workspace-id` file with the move or backup. If it was lost, restore that marker from backup before starting nanobot. | | You want one shared session across devices | Set `agents.defaults.unifiedSession` intentionally; otherwise keep separate sessions. | ## Collect Useful Evidence diff --git a/nanobot/agent/loop.py b/nanobot/agent/loop.py index 85cc23b4f..253247168 100644 --- a/nanobot/agent/loop.py +++ b/nanobot/agent/loop.py @@ -476,6 +476,12 @@ class AgentLoop: if bus is None: bus = MessageBus() defaults = config.agents.defaults + if "session_manager" not in extra: + data_dir = config.runtime_data_dir + extra["session_manager"] = SessionManager( + config.workspace_path, + sessions_root=data_dir / "sessions" if data_dir is not None else None, + ) provider = extra.pop("provider", None) or make_provider(config) resolved = config.resolve_preset() model = extra.pop("model", None) or resolved.model diff --git a/nanobot/cli/commands.py b/nanobot/cli/commands.py index 318e55b57..051037183 100644 --- a/nanobot/cli/commands.py +++ b/nanobot/cli/commands.py @@ -440,6 +440,44 @@ app.add_typer( app.command(name="agent")(agent) +# ============================================================================ +# Session Commands +# ============================================================================ + + +sessions_app = typer.Typer(help="Manage persisted session history") +app.add_typer(sessions_app, name="sessions") + + +@sessions_app.command("restore-workspace") +def sessions_restore_workspace( + config: str | None = typer.Option(None, "--config", "-c", help="Path to config file"), + workspace: str | None = typer.Option(None, "--workspace", "-w", help="Workspace directory"), +) -> None: + """Copy sessions back into the workspace before downgrading nanobot.""" + from nanobot.session.manager import SessionManager + + runtime_config = _load_runtime_config(config, workspace) + data_dir = runtime_config.runtime_data_dir + manager = SessionManager( + runtime_config.workspace_path, + sessions_root=data_dir / "sessions" if data_dir is not None else None, + ) + result = manager.restore_sessions_to_workspace() + console.print( + f"Restored {result.restored} session file(s) to " + f"{escape(str(runtime_config.workspace_path / 'sessions'))}; " + f"{result.unchanged} already matched." + ) + if result.conflicts: + console.print( + "[red]Rollback is incomplete: existing or invalid files require manual review.[/red]" + ) + for path in result.conflicts: + console.print(Text(f"- {path}", style="red")) + raise typer.Exit(1) + + # ============================================================================ # Channel Commands # ============================================================================ diff --git a/nanobot/config/loader.py b/nanobot/config/loader.py index db6522df4..f5bbcefee 100644 --- a/nanobot/config/loader.py +++ b/nanobot/config/loader.py @@ -75,6 +75,7 @@ def load_config(config_path: Path | None = None) -> Config: summary="Environment-based configuration is invalid.", issues=validation_issues(exc), ) from exc + config.bind_source_path(path) _apply_ssrf_whitelist(config) return config @@ -130,6 +131,7 @@ def load_config(config_path: Path | None = None) -> Config: issues=issues, ) from exc + config.bind_source_path(path) _apply_ssrf_whitelist(config) return config diff --git a/nanobot/config/schema.py b/nanobot/config/schema.py index 4d3d2e79c..92c97783c 100644 --- a/nanobot/config/schema.py +++ b/nanobot/config/schema.py @@ -4,7 +4,7 @@ from __future__ import annotations from pathlib import Path from typing import TYPE_CHECKING, Any, ClassVar, Literal, cast -from pydantic import AliasChoices, ConfigDict, Field, field_validator, model_validator +from pydantic import AliasChoices, ConfigDict, Field, PrivateAttr, field_validator, model_validator from pydantic_settings import BaseSettings, SettingsConfigDict from nanobot.config.timezone import detect_system_timezone @@ -431,6 +431,8 @@ class ToolsConfig(Base): class Config(BaseSettings): """Root configuration for nanobot.""" + _source_path: Path | None = PrivateAttr(default=None) + agents: AgentsConfig = Field(default_factory=AgentsConfig) channels: ChannelsConfig = Field(default_factory=ChannelsConfig) transcription: TranscriptionConfig = Field(default_factory=TranscriptionConfig) @@ -449,6 +451,15 @@ class Config(BaseSettings): _resolve_tool_config_refs() super().__init__(**values) + def bind_source_path(self, path: Path) -> None: + """Record the config file that owns instance-level runtime data.""" + self._source_path = path.expanduser().resolve(strict=False) + + @property + def runtime_data_dir(self) -> Path | None: + """Return the active instance data directory when loaded from a config path.""" + return self._source_path.parent if self._source_path is not None else None + @model_validator(mode="after") def _validate_model_preset(self) -> "Config": if "default" in self.model_presets: diff --git a/nanobot/session/manager.py b/nanobot/session/manager.py index 83498bc2e..d703f4ff5 100644 --- a/nanobot/session/manager.py +++ b/nanobot/session/manager.py @@ -6,7 +6,8 @@ import hashlib import json import os import re -import shutil +import secrets +import stat from collections import OrderedDict from contextlib import suppress from copy import deepcopy @@ -16,9 +17,10 @@ from pathlib import Path from typing import Any, Callable, Collection, Protocol, TypedDict, cast from weakref import WeakValueDictionary +from filelock import FileLock from loguru import logger -from nanobot.config.paths import get_legacy_sessions_dir +from nanobot.config.paths import get_legacy_sessions_dir, get_runtime_subdir from nanobot.providers.base import ProviderConversationState from nanobot.runtime_context import ( RUNTIME_CONTEXT_HISTORY_META, @@ -59,6 +61,11 @@ _FORK_VOLATILE_METADATA_KEYS = { "title", "title_user_edited", } +_WORKSPACE_STATE_DIR = ".nanobot" +_WORKSPACE_ID_FILE = "workspace-id" +_WORKSPACE_ID_RE = re.compile(r"^[0-9a-f]{32}$") +_SESSION_MIGRATION_LOCK_TIMEOUT_SECONDS = 30 +_COPY_CHUNK_SIZE = 1024 * 1024 def _json_object(value: object) -> dict[str, Any]: @@ -505,6 +512,23 @@ class SessionInfo(TypedDict): path: str +@dataclass(frozen=True) +class _SessionFileSnapshot: + digest: str + size: int + mtime_ns: int + updated_at: float + device: int + inode: int + + +@dataclass(frozen=True) +class SessionRestoreResult: + restored: int + unchanged: int + conflicts: tuple[Path, ...] + + class SessionStore(Protocol): def load(self, key: str) -> Session | None: ... @@ -522,34 +546,356 @@ class SessionStore(Protocol): class JsonlSessionStore: """JSONL implementation of session persistence.""" - def __init__(self, workspace: Path): - root = get_legacy_sessions_dir() - self.sessions_dir = ensure_dir(root / self._workspace_hash(workspace)) - self.legacy_sessions_dir = root - self._write_workspace_marker(self.sessions_dir, workspace) - self._migrate_from_workspace(workspace) - - @staticmethod - def _workspace_hash(workspace: Path) -> str: - canonical = str(Path(workspace).expanduser().resolve(strict=False)) - return hashlib.sha256(canonical.encode("utf-8")).hexdigest()[:16] - - @staticmethod - def _write_workspace_marker(sessions_dir: Path, workspace: Path) -> None: - marker = sessions_dir / ".workspace" - if marker.exists(): - return - try: - marker.write_text( - str(Path(workspace).expanduser().resolve(strict=False)), - encoding="utf-8", + def __init__(self, workspace: Path, *, sessions_root: Path | None = None): + canonical_workspace = Path(workspace).expanduser().resolve(strict=False) + ensure_dir(canonical_workspace) + root = ( + Path(sessions_root).expanduser().resolve(strict=False) + if sessions_root is not None + else get_runtime_subdir("sessions").resolve(strict=False) + ) + if root == canonical_workspace or root.is_relative_to(canonical_workspace): + raise RuntimeError( + "session storage must be outside the agent workspace; " + "move --config outside --workspace or choose a nested workspace directory" ) - except OSError as exc: - logger.debug("Failed to write sessions workspace marker: {}", exc) + ensure_dir(root) + with suppress(OSError): + os.chmod(root, 0o700) + self.workspace = canonical_workspace + self._migration_lock = FileLock( + str(root / ".workspace-migration.lock"), + timeout=_SESSION_MIGRATION_LOCK_TIMEOUT_SECONDS, + ) + with self._migration_lock: + workspace_id = self._load_or_create_workspace_id(canonical_workspace, root) + workspace_id = self._claim_workspace_namespace( + root, + canonical_workspace, + workspace_id, + ) + self.sessions_dir = ensure_dir(root / workspace_id) + self.legacy_sessions_dir = get_legacy_sessions_dir() + self._migrate_from_workspace(canonical_workspace) + + @staticmethod + def _fsync_directory(path: Path) -> None: + with suppress(PermissionError, NotImplementedError): + fd = os.open(path, os.O_RDONLY) + try: + os.fsync(fd) + except OSError as exc: + if exc.errno != errno.EINVAL: + raise + finally: + os.close(fd) + + @classmethod + def _write_text_atomic(cls, path: Path, content: str, *, mode: int = 0o600) -> None: + tmp = path.with_name(f".{path.name}.{secrets.token_hex(8)}.tmp") + try: + with open(tmp, "x", encoding="utf-8") as handle: + os.chmod(tmp, mode) + handle.write(content) + handle.flush() + os.fsync(handle.fileno()) + os.replace(tmp, path) + cls._fsync_directory(path.parent) + finally: + tmp.unlink(missing_ok=True) + + @classmethod + def _read_workspace_id(cls, marker: Path) -> str: + if marker.is_symlink(): + raise RuntimeError(f"workspace identity marker must not be a symlink: {marker}") + value = marker.read_text(encoding="utf-8").strip() + if not _WORKSPACE_ID_RE.fullmatch(value): + raise RuntimeError( + f"workspace identity marker is invalid: {marker}; " + "restore its original 32-character identifier before starting nanobot" + ) + return value + + @staticmethod + def _workspace_id_path(workspace: Path) -> Path: + state_dir = workspace / _WORKSPACE_STATE_DIR + if state_dir.is_symlink(): + raise RuntimeError(f"workspace state directory must not be a symlink: {state_dir}") + ensure_dir(state_dir) + return state_dir / _WORKSPACE_ID_FILE + + @classmethod + def _find_workspace_namespace(cls, workspace: Path, root: Path) -> str | None: + """Recover an identity marker removed by cleanup at the same workspace path.""" + matches: list[str] = [] + for sessions_dir in root.iterdir(): + if ( + not _WORKSPACE_ID_RE.fullmatch(sessions_dir.name) + or sessions_dir.is_symlink() + or not sessions_dir.is_dir() + ): + continue + marker = sessions_dir / ".workspace" + if marker.is_symlink() or not marker.is_file(): + continue + try: + recorded = Path(marker.read_text(encoding="utf-8").strip()).expanduser() + recorded = recorded.resolve(strict=False) + same_workspace = recorded == workspace or ( + recorded.exists() and recorded.samefile(workspace) + ) + except (OSError, UnicodeError, ValueError): + continue + if same_workspace: + matches.append(sessions_dir.name) + if len(matches) > 1: + raise RuntimeError( + f"multiple session namespaces claim workspace {workspace}; " + "remove the stale namespace marker before starting nanobot" + ) + return matches[0] if matches else None + + @classmethod + def _load_or_create_workspace_id(cls, workspace: Path, root: Path) -> str: + marker = cls._workspace_id_path(workspace) + if marker.exists() or marker.is_symlink(): + return cls._read_workspace_id(marker) + + recovered = cls._find_workspace_namespace(workspace, root) + if recovered is not None: + cls._write_text_atomic(marker, f"{recovered}\n") + return recovered + + workspace_id = secrets.token_hex(16) + flags = os.O_WRONLY | os.O_CREAT | os.O_EXCL + flags |= getattr(os, "O_NOFOLLOW", 0) + try: + fd = os.open(marker, flags, 0o600) + except FileExistsError: + return cls._read_workspace_id(marker) + try: + payload = f"{workspace_id}\n".encode("ascii") + view = memoryview(payload) + while view: + written = os.write(fd, view) + view = view[written:] + os.fsync(fd) + except BaseException: + with suppress(OSError): + marker.unlink() + raise + finally: + os.close(fd) + cls._fsync_directory(marker.parent) + return workspace_id + + @classmethod + def _replace_workspace_id(cls, workspace: Path, workspace_id: str) -> None: + cls._write_text_atomic(cls._workspace_id_path(workspace), f"{workspace_id}\n") + + @classmethod + def _write_workspace_marker(cls, sessions_dir: Path, workspace: Path) -> None: + cls._write_text_atomic(sessions_dir / ".workspace", f"{workspace}\n") + + @classmethod + def _claim_workspace_namespace( + cls, + root: Path, + workspace: Path, + workspace_id: str, + ) -> str: + """Bind a stable workspace ID, rotating copied live workspaces apart.""" + for _attempt in range(3): + sessions_dir = root / workspace_id + marker = sessions_dir / ".workspace" + if sessions_dir.is_symlink(): + raise RuntimeError(f"session namespace must not be a symlink: {sessions_dir}") + if not sessions_dir.exists(): + ensure_dir(sessions_dir) + cls._write_workspace_marker(sessions_dir, workspace) + return workspace_id + if marker.is_symlink(): + raise RuntimeError(f"session workspace marker must not be a symlink: {marker}") + if not marker.exists(): + if any(sessions_dir.iterdir()): + raise RuntimeError( + f"session namespace has data but no workspace marker: {sessions_dir}" + ) + cls._write_workspace_marker(sessions_dir, workspace) + return workspace_id + + recorded_text = marker.read_text(encoding="utf-8").strip() + if not recorded_text: + raise RuntimeError(f"session workspace marker is empty: {marker}") + recorded = Path(recorded_text).expanduser().resolve(strict=False) + if recorded == workspace: + return workspace_id + try: + same_workspace = recorded.exists() and recorded.samefile(workspace) + except OSError: + same_workspace = False + if same_workspace: + cls._write_workspace_marker(sessions_dir, workspace) + return workspace_id + if not recorded.exists(): + # The identity marker travelled with a renamed or moved workspace. + cls._write_workspace_marker(sessions_dir, workspace) + return workspace_id + + # Both paths exist and are different: this is a copy, not a move. + workspace_id = secrets.token_hex(16) + cls._replace_workspace_id(workspace, workspace_id) + + raise RuntimeError(f"could not allocate an isolated session namespace for {workspace}") + + @staticmethod + def _session_file_snapshot(path: Path) -> _SessionFileSnapshot | None: + flags = os.O_RDONLY | getattr(os, "O_NOFOLLOW", 0) + try: + fd = os.open(path, flags) + except OSError: + return None + try: + before = os.fstat(fd) + if not stat.S_ISREG(before.st_mode): + return None + digest = hashlib.sha256() + saw_record = False + updated_at: float | None = None + with os.fdopen(fd, "rb", closefd=False) as handle: + for raw_line in handle: + digest.update(raw_line) + if not raw_line.strip(): + continue + value: object = json.loads(raw_line.decode("utf-8")) + data = _json_object(value) + saw_record = True + if data.get("_type") == "metadata": + raw_updated_at = cast(object, data.get("updated_at")) + if isinstance(raw_updated_at, str) and raw_updated_at: + updated_at = datetime.fromisoformat(raw_updated_at).timestamp() + after = os.fstat(fd) + if ( + not saw_record + or before.st_dev != after.st_dev + or before.st_ino != after.st_ino + or before.st_size != after.st_size + or before.st_mtime_ns != after.st_mtime_ns + ): + return None + return _SessionFileSnapshot( + digest=digest.hexdigest(), + size=after.st_size, + mtime_ns=after.st_mtime_ns, + updated_at=(updated_at if updated_at is not None else after.st_mtime_ns / 1e9), + device=after.st_dev, + inode=after.st_ino, + ) + except (OSError, UnicodeDecodeError, json.JSONDecodeError, ValueError, TypeError): + return None + finally: + os.close(fd) + + @classmethod + def _prepare_copy( + cls, + src: Path, + dst_dir: Path, + snapshot: _SessionFileSnapshot, + ) -> Path: + tmp = dst_dir / f".{src.name}.{secrets.token_hex(8)}.tmp" + flags = os.O_RDONLY | getattr(os, "O_NOFOLLOW", 0) + src_fd = os.open(src, flags) + try: + before = os.fstat(src_fd) + if ( + before.st_dev != snapshot.device + or before.st_ino != snapshot.inode + or before.st_size != snapshot.size + or before.st_mtime_ns != snapshot.mtime_ns + ): + raise OSError("session source changed before migration") + digest = hashlib.sha256() + size = 0 + with os.fdopen(src_fd, "rb", closefd=False) as source, open(tmp, "xb") as target: + os.chmod(tmp, 0o600) + while chunk := source.read(_COPY_CHUNK_SIZE): + digest.update(chunk) + size += len(chunk) + target.write(chunk) + target.flush() + os.fsync(target.fileno()) + after = os.fstat(src_fd) + if ( + digest.hexdigest() != snapshot.digest + or size != snapshot.size + or after.st_dev != snapshot.device + or after.st_ino != snapshot.inode + or after.st_size != snapshot.size + or after.st_mtime_ns != snapshot.mtime_ns + ): + raise OSError("session source changed during migration") + return tmp + except BaseException: + tmp.unlink(missing_ok=True) + raise + finally: + os.close(src_fd) + + @classmethod + def _install_snapshot( + cls, + src: Path, + dst: Path, + snapshot: _SessionFileSnapshot, + ) -> None: + tmp = cls._prepare_copy(src, dst.parent, snapshot) + try: + os.replace(tmp, dst) + cls._fsync_directory(dst.parent) + installed = cls._session_file_snapshot(dst) + if installed is None or installed.digest != snapshot.digest: + raise OSError(f"session migration verification failed: {dst}") + finally: + tmp.unlink(missing_ok=True) + + def _archive_conflict( + self, + src: Path, + snapshot: _SessionFileSnapshot, + label: str, + ) -> Path: + conflict_dir = ensure_dir(self.sessions_dir / ".migration-conflicts") + conflict = conflict_dir / ( + f"{src.stem}.{label}.{snapshot.digest[:12]}.{secrets.token_hex(4)}.jsonl" + ) + self._install_snapshot(src, conflict, snapshot) + return conflict + + @classmethod + def _remove_migrated_source( + cls, + src: Path, + snapshot: _SessionFileSnapshot, + ) -> bool: + try: + current = src.stat(follow_symlinks=False) + if ( + current.st_dev != snapshot.device + or current.st_ino != snapshot.inode + or current.st_size != snapshot.size + or current.st_mtime_ns != snapshot.mtime_ns + ): + return False + src.unlink() + cls._fsync_directory(src.parent) + return True + except OSError: + return False def _migrate_from_workspace(self, workspace: Path) -> None: - """Move legacy in-workspace session files into the out-of-workspace store.""" - old_dir = Path(workspace).expanduser() / "sessions" + """Durably copy legacy sessions out of the workspace, then remove the source.""" + old_dir = workspace / "sessions" if old_dir.is_symlink() or not old_dir.is_dir(): if old_dir.is_symlink(): logger.warning("Skipping symlinked legacy sessions directory: {}", old_dir) @@ -559,13 +905,87 @@ class JsonlSessionStore: logger.warning("Skipping unsafe legacy session file: {}", src) continue dst = self.sessions_dir / src.name - if dst.exists(): + source_snapshot = self._session_file_snapshot(src) + if source_snapshot is None: + logger.warning("Skipping invalid or changing legacy session file: {}", src) continue try: - shutil.move(str(src), str(dst)) + destination_snapshot = self._session_file_snapshot(dst) if dst.exists() else None + if dst.exists() and destination_snapshot is None: + logger.warning( + "Keeping legacy session because destination is invalid: {}", + dst, + ) + continue + + if destination_snapshot is None: + self._install_snapshot(src, dst, source_snapshot) + elif destination_snapshot.digest == source_snapshot.digest: + pass + elif source_snapshot.updated_at > destination_snapshot.updated_at: + archived = self._archive_conflict(dst, destination_snapshot, "destination") + self._install_snapshot(src, dst, source_snapshot) + logger.warning("Archived older session migration conflict at {}", archived) + else: + archived = self._archive_conflict(src, source_snapshot, "workspace") + logger.warning("Archived older session migration conflict at {}", archived) + + installed = self._session_file_snapshot(dst) + if installed is None: + raise OSError(f"session migration destination is unreadable: {dst}") + selected_digest = ( + source_snapshot.digest + if destination_snapshot is None + or source_snapshot.updated_at > destination_snapshot.updated_at + else destination_snapshot.digest + ) + if installed.digest != selected_digest: + raise OSError(f"session migration selected unexpected data: {dst}") + if not self._remove_migrated_source(src, source_snapshot): + logger.warning( + "Session migrated but legacy source changed or could not be removed: {}", + src, + ) except OSError as exc: logger.warning("Failed to migrate session {}: {}", src, exc) + def restore_to_workspace(self) -> SessionRestoreResult: + """Copy canonical sessions back for an explicit downgrade or rollback.""" + restored = 0 + unchanged = 0 + conflicts: list[Path] = [] + old_dir = self.workspace / "sessions" + if old_dir.is_symlink(): + raise RuntimeError(f"refusing to restore into symlinked sessions directory: {old_dir}") + ensure_dir(old_dir) + + with self._migration_lock: + for src in self.sessions_dir.glob("*.jsonl"): + if self.session_key_from_path(src) is None: + continue + source_snapshot = self._session_file_snapshot(src) + if source_snapshot is None: + conflicts.append(src) + continue + dst = old_dir / src.name + if dst.exists(): + destination_snapshot = self._session_file_snapshot(dst) + if ( + destination_snapshot is not None + and destination_snapshot.digest == source_snapshot.digest + ): + unchanged += 1 + else: + conflicts.append(dst) + continue + self._install_snapshot(src, dst, source_snapshot) + restored += 1 + return SessionRestoreResult( + restored=restored, + unchanged=unchanged, + conflicts=tuple(conflicts), + ) + @staticmethod def safe_key(key: str) -> str: return safe_filename(key.replace(":", "_")) @@ -1033,9 +1453,15 @@ class JsonlSessionStore: class SessionManager: """Manage session identity, caching, retention, and persistence.""" - def __init__(self, workspace: Path, *, store: SessionStore | None = None): + def __init__( + self, + workspace: Path, + *, + store: SessionStore | None = None, + sessions_root: Path | None = None, + ): self.workspace = workspace - self._jsonl_store = JsonlSessionStore(workspace) + self._jsonl_store = JsonlSessionStore(workspace, sessions_root=sessions_root) self._store: SessionStore = store if store is not None else self._jsonl_store self.sessions_dir = self._jsonl_store.sessions_dir self.legacy_sessions_dir = self._jsonl_store.legacy_sessions_dir @@ -1201,6 +1627,10 @@ class SessionManager: self.invalidate(key) return self._store.delete(key) + def restore_sessions_to_workspace(self) -> SessionRestoreResult: + """Restore session files to the pre-relocation path for an explicit rollback.""" + return self._jsonl_store.restore_to_workspace() + def fork_session_before_user_index( self, source_key: str, diff --git a/tests/cli/test_session_restore.py b/tests/cli/test_session_restore.py new file mode 100644 index 000000000..7935d54a7 --- /dev/null +++ b/tests/cli/test_session_restore.py @@ -0,0 +1,31 @@ +from pathlib import Path + +from typer.testing import CliRunner + +from nanobot.cli import commands +from nanobot.config.loader import load_config +from nanobot.session.manager import SessionManager + + +def test_sessions_restore_workspace_command_prepares_downgrade( + tmp_path: Path, + monkeypatch, +) -> None: + workspace = tmp_path / "workspace" + config_path = tmp_path / "instance" / "config.json" + config = load_config(config_path) + config.agents.defaults.workspace = str(workspace) + manager = SessionManager(workspace, sessions_root=config_path.parent / "sessions") + session = manager.get_or_create("cli:rollback") + session.add_message("user", "restore-me") + manager.save(session, fsync=True) + monkeypatch.setattr(commands, "_load_runtime_config", lambda *_args: config) + + result = CliRunner().invoke(commands.app, ["sessions", "restore-workspace"]) + + assert result.exit_code == 0, result.output + assert "Restored 1 session file(s)" in result.output + restored = workspace / "sessions" / manager._get_session_path(session.key).name + assert restored.exists() + assert "restore-me" in restored.read_text(encoding="utf-8") + assert manager._get_session_path(session.key).exists() diff --git a/tests/config/test_config_load_errors.py b/tests/config/test_config_load_errors.py index b5aeab83d..db08e9035 100644 --- a/tests/config/test_config_load_errors.py +++ b/tests/config/test_config_load_errors.py @@ -3,14 +3,29 @@ import json import pytest from nanobot.config.errors import ConfigLoadError -from nanobot.config.loader import load_config +from nanobot.config.loader import load_config, resolve_config_env_vars from nanobot.config.schema import ApiConfig def test_load_config_missing_file_uses_defaults(tmp_path) -> None: - config = load_config(tmp_path / "missing.json") + config_path = tmp_path / "instance" / "missing.json" + config = load_config(config_path) assert config.agents.defaults.model + assert config.runtime_data_dir == config_path.parent + + +def test_env_resolution_preserves_config_runtime_data_dir( + tmp_path, monkeypatch: pytest.MonkeyPatch +) -> None: + config_path = tmp_path / "instance" / "config.json" + config_path.parent.mkdir() + config_path.write_text('{"providers": {"openai": {"apiKey": "${TEST_API_KEY}"}}}') + monkeypatch.setenv("TEST_API_KEY", "resolved") + + config = resolve_config_env_vars(load_config(config_path), config_path=config_path) + + assert config.runtime_data_dir == config_path.parent def test_load_config_reports_malformed_environment_safely( diff --git a/tests/session/test_session_location.py b/tests/session/test_session_location.py index 07e8b48f7..c041152d4 100644 --- a/tests/session/test_session_location.py +++ b/tests/session/test_session_location.py @@ -3,14 +3,24 @@ from __future__ import annotations import json +import shutil +import uuid from pathlib import Path +from unittest.mock import patch import pytest +from nanobot.config.loader import load_config from nanobot.session.manager import JsonlSessionStore, SessionManager -def _write_legacy_session(old_dir: Path, key: str, content: str) -> Path: +def _write_legacy_session( + old_dir: Path, + key: str, + content: str, + *, + updated_at: str = "2026-01-01T00:00:00", +) -> Path: """Write a valid session file in the legacy in-workspace location.""" old_dir.mkdir(parents=True, exist_ok=True) path = old_dir / f"{JsonlSessionStore.storage_key(key)}.jsonl" @@ -20,7 +30,7 @@ def _write_legacy_session(old_dir: Path, key: str, content: str) -> Path: "_type": "metadata", "key": key, "created_at": "2026-01-01T00:00:00", - "updated_at": "2026-01-01T00:00:00", + "updated_at": updated_at, "metadata": {}, "last_consolidated": 0, } @@ -45,15 +55,37 @@ def test_sessions_are_stored_outside_workspace(tmp_path: Path) -> None: workspace_sessions = workspace / "sessions" assert not workspace_sessions.exists() or not any(workspace_sessions.glob("*.jsonl")) - # The out-of-workspace store records which workspace it belongs to. + # The out-of-workspace store records which workspace it belongs to and the + # workspace carries only a non-secret stable identity marker. marker = manager.sessions_dir / ".workspace" - assert marker.read_text(encoding="utf-8") == str(workspace.resolve()) + assert marker.read_text(encoding="utf-8").strip() == str(workspace.resolve()) + workspace_id = (workspace / ".nanobot" / "workspace-id").read_text(encoding="utf-8").strip() + assert manager.sessions_dir.name == workspace_id + assert manager.sessions_dir.parent.name == "sessions" # And it must still round-trip through a fresh manager for the same workspace. reloaded = SessionManager(workspace=workspace).get_or_create("telegram:1") assert reloaded.messages[-1]["content"] == "hello" +def test_workspace_identity_marker_contains_no_session_content(tmp_path: Path) -> None: + workspace = tmp_path / "workspace" + secret = f"session-secret-{uuid.uuid4()}" + manager = SessionManager(workspace=workspace) + session = manager.get_or_create("telegram:secret") + session.add_message("user", secret) + manager.save(session) + + marker = workspace / ".nanobot" / "workspace-id" + assert marker.read_text(encoding="utf-8").strip() == manager.sessions_dir.name + assert secret not in marker.read_text(encoding="utf-8") + assert not any( + secret in path.read_text(encoding="utf-8") + for path in workspace.rglob("*") + if path.is_file() + ) + + def test_different_workspaces_are_isolated(tmp_path: Path) -> None: workspace_a = tmp_path / "ws_a" workspace_b = tmp_path / "ws_b" @@ -69,6 +101,94 @@ def test_different_workspaces_are_isolated(tmp_path: Path) -> None: assert in_b.messages == [] +def test_sessions_follow_active_custom_config_data_root( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + custom_instance = tmp_path / "instance-b" + custom_config = custom_instance / "config.json" + default_home = tmp_path / "read-only-home" + default_home.mkdir() + default_home.chmod(0o500) + monkeypatch.setenv("HOME", str(default_home)) + config = load_config(custom_config) + data_dir = config.runtime_data_dir + assert data_dir == custom_instance + + manager = SessionManager( + workspace=tmp_path / "workspace-b", + sessions_root=data_dir / "sessions", + ) + session = manager.get_or_create("telegram:custom") + session.add_message("user", "custom-instance") + manager.save(session) + + assert manager.sessions_dir.parent == custom_instance / "sessions" + assert manager._get_session_path(session.key).exists() + assert not (default_home / ".nanobot" / "sessions").exists() + + +def test_session_root_inside_workspace_fails_closed(tmp_path: Path) -> None: + workspace = tmp_path / "workspace" + + with pytest.raises(RuntimeError, match="must be outside the agent workspace"): + SessionManager(workspace=workspace, sessions_root=workspace / "sessions") + + +def test_workspace_move_preserves_session_identity(tmp_path: Path) -> None: + original = tmp_path / "project-old" + manager = SessionManager(workspace=original) + session = manager.get_or_create("telegram:1") + session.add_message("user", "survives-move") + manager.save(session) + + moved = tmp_path / "project-new" + original.rename(moved) + reloaded = SessionManager(workspace=moved) + + assert reloaded.sessions_dir == manager.sessions_dir + assert reloaded.get_or_create("telegram:1").messages[-1]["content"] == "survives-move" + assert (reloaded.sessions_dir / ".workspace").read_text(encoding="utf-8").strip() == str( + moved.resolve() + ) + + +def test_deleted_workspace_identity_marker_is_recovered(tmp_path: Path) -> None: + workspace = tmp_path / "project" + manager = SessionManager(workspace=workspace) + session = manager.get_or_create("telegram:1") + session.add_message("user", "survives-cleanup") + manager.save(session) + + shutil.rmtree(workspace / ".nanobot") + reloaded = SessionManager(workspace=workspace) + + assert reloaded.sessions_dir == manager.sessions_dir + assert reloaded.get_or_create("telegram:1").messages[-1]["content"] == "survives-cleanup" + assert (workspace / ".nanobot" / "workspace-id").read_text(encoding="utf-8").strip() == ( + manager.sessions_dir.name + ) + + +def test_copied_workspace_gets_isolated_session_identity(tmp_path: Path) -> None: + original = tmp_path / "project-a" + original.mkdir() + manager = SessionManager(workspace=original) + session = manager.get_or_create("telegram:1") + session.add_message("user", "secret-for-a") + manager.save(session) + + copied = tmp_path / "project-b" + shutil.copytree(original, copied) + copied_manager = SessionManager(workspace=copied) + + assert copied_manager.sessions_dir != manager.sessions_dir + assert copied_manager.get_or_create("telegram:1").messages == [] + assert (copied / ".nanobot" / "workspace-id").read_text(encoding="utf-8") != ( + original / ".nanobot" / "workspace-id" + ).read_text(encoding="utf-8") + + def test_equivalent_workspace_paths_share_one_store(tmp_path: Path) -> None: real_workspace = tmp_path / "real_ws" real_workspace.mkdir() @@ -105,6 +225,74 @@ def test_legacy_in_workspace_sessions_are_migrated(tmp_path: Path) -> None: assert again.messages[-1]["content"] == "migrated-msg" +def test_migration_keeps_source_when_install_fails(tmp_path: Path) -> None: + workspace = tmp_path / "workspace" + key = "telegram:partial" + old_file = _write_legacy_session(workspace / "sessions", key, "still-safe") + + with patch.object(JsonlSessionStore, "_install_snapshot", side_effect=OSError("disk full")): + manager = SessionManager(workspace=workspace) + + assert old_file.exists() + assert not (manager.sessions_dir / old_file.name).exists() + + retried = SessionManager(workspace=workspace) + assert retried.get_or_create(key).messages[-1]["content"] == "still-safe" + assert not old_file.exists() + + +def test_migration_preserves_newest_valid_conflict(tmp_path: Path) -> None: + workspace = tmp_path / "workspace" + key = "telegram:conflict" + old_file = _write_legacy_session(workspace / "sessions", key, "newer-workspace") + manager = SessionManager(workspace=workspace) + + # Recreate an older legacy source while a newer destination already exists. + manager_session = manager.get_or_create(key) + manager_session.add_message("assistant", "newer-destination") + manager.save(manager_session) + _write_legacy_session( + workspace / "sessions", + key, + "older-workspace", + updated_at="2025-01-01T00:00:00", + ) + + retried = SessionManager(workspace=workspace) + loaded = retried.get_or_create(key) + + assert loaded.messages[-1]["content"] == "newer-destination" + conflicts = list((retried.sessions_dir / ".migration-conflicts").glob("*.jsonl")) + assert len(conflicts) == 1 + assert "older-workspace" in conflicts[0].read_text(encoding="utf-8") + assert not old_file.exists() + + +def test_explicit_rollback_restore_copies_sessions_back_without_deleting_new_store( + tmp_path: Path, +) -> None: + workspace = tmp_path / "workspace" + manager = SessionManager(workspace=workspace) + session = manager.get_or_create("telegram:rollback") + session.add_message("user", "available-to-old-version") + manager.save(session, fsync=True) + + result = manager.restore_sessions_to_workspace() + legacy_file = workspace / "sessions" / manager._get_session_path(session.key).name + + assert result.restored == 1 + assert result.unchanged == 0 + assert result.conflicts == () + assert legacy_file.exists() + assert manager._get_session_path(session.key).exists() + assert "available-to-old-version" in legacy_file.read_text(encoding="utf-8") + + repeated = manager.restore_sessions_to_workspace() + assert repeated.restored == 0 + assert repeated.unchanged == 1 + assert repeated.conflicts == () + + def test_legacy_migration_rejects_symlinked_session_file(tmp_path: Path) -> None: workspace = tmp_path / "workspace" old_dir = workspace / "sessions" diff --git a/tests/test_nanobot_facade.py b/tests/test_nanobot_facade.py index 1d097ffdc..dfddf7150 100644 --- a/tests/test_nanobot_facade.py +++ b/tests/test_nanobot_facade.py @@ -46,7 +46,9 @@ def _write_config(tmp_path: Path, overrides: dict | None = None) -> Path: } if overrides: data.update(overrides) - config_path = tmp_path / "config.json" + config_dir = tmp_path.parent / f"{tmp_path.name}-instance" + config_dir.mkdir(exist_ok=True) + config_path = config_dir / "config.json" config_path.write_text(json.dumps(data)) return config_path @@ -96,9 +98,11 @@ def test_from_config_missing_env_reports_explicit_config_path( def test_from_config_creates_instance(tmp_path): config_path = _write_config(tmp_path) - bot = Nanobot.from_config(config_path, workspace=tmp_path) + workspace = tmp_path / "workspace" + bot = Nanobot.from_config(config_path, workspace=workspace) assert bot._loop is not None - assert bot._loop.workspace == tmp_path + assert bot._loop.workspace == workspace + assert bot._loop.sessions.sessions_dir.parent == config_path.parent / "sessions" def test_from_config_composes_configured_mcp_outside_agent_loop(tmp_path):