Merge remote-tracking branch 'origin/main' into nanobot-webui

nanobot/agent/tools/cron.py

@@ -5,13 +5,16 @@ from datetime import datetime
 from typing import Any
 
 from nanobot.agent.tools.base import Tool, tool_parameters
-from nanobot.agent.tools.schema import BooleanSchema, IntegerSchema, StringSchema, tool_parameters_schema
+from nanobot.agent.tools.schema import (
+    BooleanSchema,
+    IntegerSchema,
+    StringSchema,
+    tool_parameters_schema,
+)
 from nanobot.cron.service import CronService
 from nanobot.cron.types import CronJob, CronJobState, CronSchedule
 
-
+_CRON_PARAMETERS = tool_parameters_schema(
-@tool_parameters(
-    tool_parameters_schema(
     action=StringSchema("Action to perform", enum=["add", "list", "remove"]),
     name=StringSchema(
         "Optional short human-readable label for the job "
@@ -38,8 +41,35 @@ from nanobot.cron.types import CronJob, CronJobState, CronSchedule
     ),
     job_id=StringSchema("REQUIRED when action='remove'. Job ID to remove (obtain via action='list')."),
     required=["action"],
-    )
+    description=(
+        "Action-specific parameters: add requires a non-empty message plus one schedule "
+        "(every_seconds, cron_expr, or at); remove requires job_id; list only needs action."
+    ),
 )
+_CRON_PARAMETERS["oneOf"] = [
+    {
+        "properties": {
+            "action": {"enum": ["add"]},
+            "message": {"type": "string", "minLength": 1},
+        },
+        "required": ["action", "message"],
+    },
+    {
+        "properties": {
+            "action": {"enum": ["list"]},
+        },
+        "required": ["action"],
+    },
+    {
+        "properties": {
+            "action": {"enum": ["remove"]},
+        },
+        "required": ["action", "job_id"],
+    },
+]
+
+
+@tool_parameters(_CRON_PARAMETERS)
 class CronTool(Tool):
     """Tool to schedule reminders and recurring tasks."""
 
@@ -95,6 +125,15 @@ class CronTool(Tool):
             f"If tz is omitted, cron expressions and naive ISO times default to {self._default_timezone}."
         )
 
+    def validate_params(self, params: dict[str, Any]) -> list[str]:
+        errors = super().validate_params(params)
+        action = params.get("action")
+        if action == "add" and not str(params.get("message") or "").strip():
+            errors.append("message is required when action='add'")
+        if action == "remove" and not str(params.get("job_id") or "").strip():
+            errors.append("job_id is required when action='remove'")
+        return errors
+
     async def execute(
         self,
         action: str,
@@ -130,8 +169,8 @@ class CronTool(Tool):
     ) -> str:
         if not message:
             return (
-                "Error: cron action='add' requires a non-empty 'message' "
+                "Error: cron action='add' requires a non-empty 'message' parameter "
-                "parameter describing what to do when the job triggers "
+                "describing what to do when the job triggers "
                 "(e.g. the reminder text). Retry including message=\"...\"."
             )
         if not self._channel or not self._chat_id:

nanobot/cli/stream.py

@@ -18,7 +18,17 @@ from nanobot import __logo__
 
 
 def _make_console() -> Console:
-    return Console(file=sys.stdout, force_terminal=True)
+    """Create a Console that emits plain text when stdout is not a TTY.
+
+    Rich's spinner, Live render, and cursor-visibility escape codes all
+    key off ``Console.is_terminal``. Forcing ``force_terminal=True`` overrode
+    the ``isatty()`` check and caused control sequences (``\\x1b[?25l``,
+    braille spinner frames) to pollute programmatic consumers such as
+    ``docker exec -i`` or pipes, even with ``NO_COLOR`` or ``TERM=dumb``.
+    Deferring to ``isatty()`` keeps Rich output in interactive terminals
+    and plain text everywhere else (#3265).
+    """
+    return Console(file=sys.stdout, force_terminal=sys.stdout.isatty())
 
 
 class ThinkingSpinner:

nanobot/config/schema.py

@@ -319,17 +319,15 @@ class Config(BaseSettings):
         return p.api_key if p else None
 
     def get_api_base(self, model: str | None = None) -> str | None:
-        """Get API base URL for the given model. Applies default URLs for gateway/local providers."""
+        """Get API base URL for the given model, falling back to the provider default when present."""
         from nanobot.providers.registry import find_by_name
 
         p, name = self._match_provider(model)
         if p and p.api_base:
             return p.api_base
-        # Only gateways get a default api_base here. Standard providers
-        # resolve their base URL from the registry in the provider constructor.
         if name:
             spec = find_by_name(name)
-            if spec and (spec.is_gateway or spec.is_local) and spec.default_api_base:
+            if spec and spec.default_api_base:
                 return spec.default_api_base
         return None
 

nanobot/utils/gitstore.py

@@ -64,14 +64,35 @@ class GitStore:
         if self.is_initialized():
             return False
 
+        if self._is_inside_git_repo():
+            logger.warning(
+                "Workspace {} is already inside a git repo; "
+                "skipping nested repo initialization",
+                self._workspace,
+            )
+            return False
+
         try:
             from dulwich import porcelain
 
             porcelain.init(str(self._workspace))
 
-            # Write .gitignore
+            # Write .gitignore (merge with existing if present)
             gitignore = self._workspace / ".gitignore"
-            gitignore.write_text(self._build_gitignore(), encoding="utf-8")
+            dream_entries = self._build_gitignore()
+            if gitignore.exists():
+                existing = gitignore.read_text(encoding="utf-8")
+                existing_lines = set(existing.splitlines())
+                new_lines = [
+                    line
+                    for line in dream_entries.splitlines()
+                    if line not in existing_lines
+                ]
+                if new_lines:
+                    merged = existing.rstrip("\n") + "\n" + "\n".join(new_lines) + "\n"
+                    gitignore.write_text(merged, encoding="utf-8")
+            else:
+                gitignore.write_text(dream_entries, encoding="utf-8")
 
             # Ensure tracked files exist (touch them if missing) so the initial
             # commit has something to track.
@@ -155,6 +176,22 @@ class GitStore:
         except Exception:
             return None
 
+    def _is_inside_git_repo(self) -> bool:
+        """Check if self._workspace is already inside a git repository.
+
+        Walks up from self._workspace to the filesystem root, returning True
+        if any parent directory contains a .git entry.
+
+        Git worktrees and submodules can use a ``.git`` file instead of a
+        directory, so we must treat either form as "already inside a repo".
+        """
+        current = self._workspace.resolve()
+        while current != current.parent:
+            if (current / ".git").exists():
+                return True
+            current = current.parent
+        return False
+
     def _build_gitignore(self) -> str:
         """Generate .gitignore content from tracked files."""
         dirs: set[str] = set()

tests/cli/test_cli_input.py

@@ -167,7 +167,19 @@ def test_stream_renderer_stop_for_input_stops_spinner():
         spinner.stop.assert_called_once()
 
 
-def test_make_console_uses_force_terminal():
+def test_make_console_force_terminal_when_stdout_is_tty():
-    """Console should be created with force_terminal=True for proper ANSI handling."""
+    """Console should set force_terminal=True when stdout is a TTY (rich output)."""
+    import sys
+    with patch.object(sys.stdout, "isatty", return_value=True):
         console = stream_mod._make_console()
         assert console._force_terminal is True
+
+
+def test_make_console_force_terminal_false_when_stdout_is_not_tty():
+    """Console should set force_terminal=False when stdout is not a TTY so that
+    ANSI escape codes (cursor visibility, braille spinner frames) don't pollute
+    piped output such as `docker exec -i` (#3265)."""
+    import sys
+    with patch.object(sys.stdout, "isatty", return_value=False):
+        console = stream_mod._make_console()
+        assert console._force_terminal is False

tests/cli/test_commands.py

@@ -286,6 +286,39 @@ def test_find_by_name_accepts_camel_case_and_hyphen_aliases():
     assert find_by_name("github-copilot").name == "github_copilot"
 
 
+def test_config_explicit_xiaomi_mimo_provider_uses_default_api_base():
+    config = Config.model_validate(
+        {
+            "agents": {
+                "defaults": {
+                    "provider": "xiaomi_mimo",
+                    "model": "MiniMax-M1-80k",
+                }
+            },
+            "providers": {
+                "xiaomiMimo": {
+                    "apiKey": "test-key",
+                }
+            },
+        }
+    )
+
+    assert config.get_provider_name() == "xiaomi_mimo"
+    assert config.get_api_base() == "https://api.xiaomimimo.com/v1"
+
+
+def test_config_auto_detects_xiaomi_mimo_from_model_keyword():
+    config = Config.model_validate(
+        {
+            "agents": {"defaults": {"provider": "auto", "model": "mimo/MiniMax-M1-80k"}},
+            "providers": {"xiaomiMimo": {"apiKey": "test-key"}},
+        }
+    )
+
+    assert config.get_provider_name() == "xiaomi_mimo"
+    assert config.get_api_base() == "https://api.xiaomimimo.com/v1"
+
+
 def test_config_auto_detects_ollama_from_local_api_base():
     config = Config.model_validate(
         {

tests/cron/test_cron_tool_list.py

@@ -7,7 +7,6 @@ import pytest
 from nanobot.agent.tools.cron import CronTool
 from nanobot.cron.service import CronService
 from nanobot.cron.types import CronJob, CronJobState, CronPayload, CronSchedule
-from tests.test_openai_api import pytest_plugins
 
 
 def _make_tool(tmp_path) -> CronTool:
@@ -346,6 +345,47 @@ def test_add_job_can_disable_delivery(tmp_path) -> None:
     assert job.payload.deliver is False
 
 
+def test_cron_schema_advertises_action_specific_requirements(tmp_path) -> None:
+    tool = _make_tool(tmp_path)
+
+    assert tool.parameters["required"] == ["action"]
+    assert tool.parameters["oneOf"] == [
+        {
+            "properties": {
+                "action": {"enum": ["add"]},
+                "message": {"type": "string", "minLength": 1},
+            },
+            "required": ["action", "message"],
+        },
+        {
+            "properties": {"action": {"enum": ["list"]}},
+            "required": ["action"],
+        },
+        {
+            "properties": {"action": {"enum": ["remove"]}},
+            "required": ["action", "job_id"],
+        },
+    ]
+
+
+def test_validate_params_requires_message_only_for_add(tmp_path) -> None:
+    tool = _make_tool(tmp_path)
+
+    assert "message is required when action='add'" in tool.validate_params({"action": "add"})
+    assert tool.validate_params({"action": "list"}) == []
+    assert "job_id is required when action='remove'" in tool.validate_params({"action": "remove"})
+
+
+def test_add_job_empty_message_returns_actionable_error(tmp_path) -> None:
+    tool = _make_tool(tmp_path)
+    tool.set_context("telegram", "chat-1")
+
+    result = tool._add_job(None, "", 60, None, None, None)
+
+    assert "action='add' requires a non-empty 'message'" in result
+    assert "Retry including message=" in result
+
+
 def test_list_excludes_disabled_jobs(tmp_path) -> None:
     tool = _make_tool(tmp_path)
     job = tool._cron.add_job(

tests/utils/test_gitstore.py

@@ -1,7 +1,8 @@
 """Tests for GitStore — line_ages() and core git operations."""
 
+import subprocess
 import time
-from datetime import datetime, timezone, timedelta
+from datetime import datetime, timedelta, timezone
 from unittest.mock import patch
 
 import pytest
@@ -89,3 +90,127 @@ class TestLineAges:
         # "- new" line and "- keep" line both age=0 (same day), but
         # the key point is we get per-line results
         assert len(ages) == 7
+
+
+class TestNestedRepoProtection:
+    """Regression tests for GitHub issue #2980: nested repo protection."""
+
+    def test_init_refuses_inside_git_repo(self, tmp_path):
+        """init() should detect it's inside an existing git repo and refuse."""
+        project = tmp_path / "project"
+        project.mkdir()
+        (project / ".git").mkdir()
+
+        workspace = project / "workspace"
+        workspace.mkdir()
+
+        g = GitStore(workspace, tracked_files=["MEMORY.md"])
+        result = g.init()
+
+        assert result is False
+        assert not (workspace / ".git").is_dir()
+
+    def test_init_preserves_existing_gitignore(self, tmp_path):
+        """init() should preserve existing .gitignore entries and append new ones."""
+        workspace = tmp_path / "workspace"
+        workspace.mkdir()
+
+        existing = "*.pyc\n__pycache__/\n"
+        (workspace / ".gitignore").write_text(existing, encoding="utf-8")
+
+        g = GitStore(workspace, tracked_files=["MEMORY.md"])
+        result = g.init()
+
+        assert result is True
+        gitignore = (workspace / ".gitignore").read_text(encoding="utf-8")
+        assert "*.pyc" in gitignore
+        assert "__pycache__/" in gitignore
+        assert "!MEMORY.md" in gitignore
+        assert "!.gitignore" in gitignore
+
+    def test_init_no_gitignore_creates_new(self, tmp_path):
+        """init() should create .gitignore with Dream content when none exists."""
+        workspace = tmp_path / "workspace"
+        workspace.mkdir()
+
+        g = GitStore(workspace, tracked_files=["MEMORY.md"])
+        result = g.init()
+
+        assert result is True
+        gitignore = (workspace / ".gitignore").read_text(encoding="utf-8")
+        expected = g._build_gitignore()
+        assert gitignore == expected
+
+    def test_init_gitignore_merge_idempotent(self, tmp_path):
+        """init() should not duplicate Dream entries already in .gitignore."""
+        workspace = tmp_path / "workspace"
+        workspace.mkdir()
+
+        # Pre-existing .gitignore that already has some Dream entries
+        existing = "*.pyc\n/*\n!MEMORY.md\n"
+        (workspace / ".gitignore").write_text(existing, encoding="utf-8")
+
+        g = GitStore(workspace, tracked_files=["MEMORY.md"])
+        result = g.init()
+
+        assert result is True
+        gitignore = (workspace / ".gitignore").read_text(encoding="utf-8")
+        # No duplicate lines
+        lines = gitignore.splitlines()
+        assert lines.count("/*") == 1
+        assert lines.count("!MEMORY.md") == 1
+        # Existing entry preserved, new Dream entries appended
+        assert "*.pyc" in gitignore
+        assert "!.gitignore" in gitignore
+
+    def test_init_outside_git_repo_works_normally(self, tmp_path):
+        """init() should succeed and create .git when not inside a git repo."""
+        workspace = tmp_path / "workspace"
+        workspace.mkdir()
+
+        g = GitStore(workspace, tracked_files=["MEMORY.md"])
+        result = g.init()
+
+        assert result is True
+        assert (workspace / ".git").is_dir()
+
+    def test_init_refuses_inside_git_worktree(self, tmp_path):
+        """init() should refuse when the parent checkout is a git worktree."""
+        repo = tmp_path / "repo"
+        repo.mkdir()
+        subprocess.run(["git", "init", "-q", str(repo)], check=True)
+        (repo / "README.md").write_text("x\n", encoding="utf-8")
+        subprocess.run(["git", "-C", str(repo), "add", "README.md"], check=True)
+        subprocess.run(
+            [
+                "git",
+                "-C",
+                str(repo),
+                "-c",
+                "user.name=test",
+                "-c",
+                "user.email=test@example.com",
+                "commit",
+                "-q",
+                "-m",
+                "init",
+            ],
+            check=True,
+        )
+        subprocess.run(["git", "-C", str(repo), "branch", "wt-branch"], check=True)
+
+        worktree = tmp_path / "worktree"
+        subprocess.run(
+            ["git", "-C", str(repo), "worktree", "add", "-q", str(worktree), "wt-branch"],
+            check=True,
+        )
+        assert (worktree / ".git").is_file()
+
+        workspace = worktree / "workspace"
+        workspace.mkdir()
+
+        g = GitStore(workspace, tracked_files=["MEMORY.md"])
+        result = g.init()
+
+        assert result is False
+        assert not (workspace / ".git").exists()