diff --git a/contree_cli/__main__.py b/contree_cli/__main__.py index 27ce8a0..223d4e9 100644 --- a/contree_cli/__main__.py +++ b/contree_cli/__main__.py @@ -8,7 +8,7 @@ from contree_cli import CLIENT, FORMATTER, PROFILE, SESSION_STORE, ArgumentsProtocol from contree_cli.arguments import parser from contree_cli.client import ApiError, client_from_profile -from contree_cli.config import Config +from contree_cli.config import SETTINGS, Config from contree_cli.log import setup_logging from contree_cli.output import FORMATTERS from contree_cli.session import SessionStore, get_session_key @@ -21,6 +21,9 @@ def main() -> None: parser.print_help() exit(0) + if SETTINGS.has_section("cli"): + parser.set_defaults(**SETTINGS["cli"]) + args = parser.parse_args() setup_logging(level=getattr(logging, args.log_level.upper(), logging.INFO)) @@ -38,14 +41,19 @@ def main() -> None: if args.project: profile = replace(profile, project=args.project) - # auth creates its own client and can work with nonexistent profiles - if args.command not in ("auth",): - if profile.name not in cfg: - log.error( - "Profile %r does not exist. Run `contree auth` first.", - profile.name, - ) - exit(1) + # Local-only commands don't need a client or a configured profile: + # auth bootstraps its own; agent/man/skill operate purely on local files. + LOCAL_COMMANDS = ("auth", "agent", "man", "skill") + needs_client = args.command not in LOCAL_COMMANDS + + if needs_client and profile.name not in cfg: + log.error( + "Profile %r does not exist. Run `contree auth` first.", + profile.name, + ) + exit(1) + + if needs_client: try: client = client_from_profile(profile) except ValueError as exc: @@ -57,6 +65,7 @@ def main() -> None: session_key = get_session_key(profile.name, override=args.session_key) db_path = profile.session_db_path + log.debug("Running in session: %s", session_key) with SessionStore(db_path, session_key) as store: PROFILE.set(profile) diff --git a/contree_cli/arguments.py b/contree_cli/arguments.py index 83b9312..acb7417 100644 --- a/contree_cli/arguments.py +++ b/contree_cli/arguments.py @@ -95,7 +95,7 @@ Authentication: Bearer token + project ID. Default API URL: - https://api.studio.nebius.com/sandboxes/ + https://api.tokenfactory.nebius.com/sandboxes/ Use `contree auth --help` to configure persistent credentials. diff --git a/contree_cli/cli/file.py b/contree_cli/cli/file.py index 8d30868..9f42648 100644 --- a/contree_cli/cli/file.py +++ b/contree_cli/cli/file.py @@ -19,7 +19,6 @@ import hashlib import json import logging -import os import shlex import subprocess import tempfile @@ -28,6 +27,7 @@ from contree_cli import CLIENT, SESSION_STORE, ArgumentsProtocol, SetupResult from contree_cli.client import ApiError, ContreeClient, resolve_image, stream_response +from contree_cli.config import EDITOR from contree_cli.session import SessionStore from contree_cli.types import FLAGS @@ -79,7 +79,8 @@ def setup_parser(p: argparse.ArgumentParser) -> SetupResult: ) edit_p.add_argument( *FLAGS["editor"], - help="Editor command (default: $EDITOR or vi)", + default=EDITOR, + help=f"Editor command (default: {EDITOR})", ) edit_p.add_argument("path", help="Path inside image") edit_p.set_defaults(handler=cmd_file_edit, load_args=FileEditArgs) @@ -128,14 +129,14 @@ def _upload_and_record( except ApiError as exc: if exc.status != 404: raise - data = local_path.read_bytes() - resp = client.request( - "POST", - "/v1/files", - body=data, - headers={"Content-Type": "application/octet-stream"}, - ) - file_uuid = json.loads(resp.read())["uuid"] + with open(local_path, "rb") as fh: + resp = client.request( + "POST", + "/v1/files", + body=fh, + headers={"Content-Type": "application/octet-stream"}, + ) + file_uuid = json.loads(resp.read())["uuid"] logger.info("Uploaded %s (%s)", instance_path, file_uuid) history_id = store.set_image( @@ -178,14 +179,13 @@ def cmd_file_edit(args: FileEditArgs) -> int | None: # 2. Record original hash, open editor original_hash = _file_sha256(tmp_file) - editor = args.editor or os.environ.get("EDITOR", "vi") - logger.info("Opening %s in %s", tmp_file, editor) + logger.info("Opening %s in %s", tmp_file, args.editor) # $EDITOR may contain shell expressions (env vars, tilde, pipes), # e.g. "TERM=xterm vim" or "~/bin/editor". shlex.split would not - # expand those — shell=True is required. The file path is quoted + # expand those, shell=True is required. The file path is quoted # via shlex.quote to prevent injection from the filename. # nosemgrep: subprocess-shell-true - rc = subprocess.call(f"{editor} {shlex.quote(str(tmp_file))}", shell=True) + rc = subprocess.call(f"{args.editor} {shlex.quote(str(tmp_file))}", shell=True) if rc != 0: logger.error("Editor exited with code %d", rc) return 1 diff --git a/contree_cli/cli/run.py b/contree_cli/cli/run.py index be4fede..b1ec395 100644 --- a/contree_cli/cli/run.py +++ b/contree_cli/cli/run.py @@ -49,17 +49,20 @@ import argparse import base64 import fnmatch +import functools import io import json import logging import os import re import select +import shlex import sys import time import uuid from dataclasses import dataclass, field from datetime import timedelta +from multiprocessing.pool import ThreadPool from contree_cli import CLIENT, FORMATTER, SESSION_STORE, ArgumentsProtocol, SetupResult from contree_cli.client import ApiError, ContreeClient, decode_stream, resolve_image @@ -68,7 +71,7 @@ DefaultFormatter, OutputFormatter, ) -from contree_cli.session import SessionStore +from contree_cli.session import CONTREE_CONCURRENCY, SessionStore from contree_cli.types import FLAGS logger = logging.getLogger(__name__) @@ -351,47 +354,82 @@ def _local_file_cache_kind(host_path: str) -> str: return f"local_file:{digest}" -MAX_CACHE_AGE = 90 * 24 * 3600 # 90 days — server retention is 6 months +MAX_CACHE_AGE = 90 * 24 * 3600 # 90 days - server retention is 6 months -def _upload_file( - client: ContreeClient, - mf: MappedFile, - store: SessionStore, -) -> str: - """Upload a host file and return its UUID, reusing if already present.""" +def cached_local_uuid(mf: MappedFile, store: SessionStore) -> str | None: + """Return a cached UUID for *mf* if one was uploaded within MAX_CACHE_AGE.""" cache_kind = _local_file_cache_kind(mf.host_path) cached = store.cache.get(("", cache_kind)) if isinstance(cached, dict) and cached.get("uuid"): age = time.time() - cached.get("uploaded_at", 0) if age < MAX_CACHE_AGE: logger.debug( - "File %s reused from local cache (%s)", mf.host_path, cached["uuid"] + "File %s reused from local cache (%s)", + mf.host_path, + cached["uuid"], ) return str(cached["uuid"]) + return None + + +def record_local_uuid(mf: MappedFile, file_uuid: str, store: SessionStore) -> None: + """Persist a host_path → file_uuid mapping in the local cache.""" + cache_kind = _local_file_cache_kind(mf.host_path) + store.cache[("", cache_kind)] = { + "uuid": file_uuid, + "uploaded_at": time.time(), + } + +def upload_one_remote(client: ContreeClient, mf: MappedFile) -> tuple[MappedFile, str]: + """HTTP-only upload (sha256 dedup + POST /v1/files). Thread-safe.""" try: resp = client.get("/v1/files", params={"sha256": mf.sha256()}) file_uuid = str(json.loads(resp.read())["uuid"]) - logger.info("File %s already uploaded (%s)", mf.host_path, file_uuid) - store.cache[("", cache_kind)] = {"uuid": file_uuid, "uploaded_at": time.time()} - return file_uuid + logger.info("Uploaded file: %s -> %s", mf.host_path, file_uuid) + return mf, file_uuid except ApiError as exc: if exc.status != 404: raise with open(mf.host_path, "rb") as fh: - data = fh.read() - resp = client.request( - "POST", - "/v1/files", - body=data, - headers={"Content-Type": "application/octet-stream"}, - ) - file_uuid = str(json.loads(resp.read())["uuid"]) + resp = client.request( + "POST", + "/v1/files", + body=fh, + headers={"Content-Type": "application/octet-stream"}, + ) + file_uuid = str(json.loads(resp.read())["uuid"]) logger.debug("Uploaded %s (%s)", mf.host_path, file_uuid) - store.cache[("", cache_kind)] = {"uuid": file_uuid, "uploaded_at": time.time()} - return file_uuid + return mf, file_uuid + + +def upload_files( + client: ContreeClient, + files: list[MappedFile], + store: SessionStore, +) -> dict[str, str]: + """Upload host files in parallel, returning host_path → file_uuid.""" + uploaded: dict[str, str] = {} + pending: list[MappedFile] = [] + for mf in files: + cached = cached_local_uuid(mf, store) + if cached: + uploaded[mf.host_path] = cached + else: + pending.append(mf) + + if not pending: + return uploaded + + workers = min(CONTREE_CONCURRENCY, len(pending)) + upload = functools.partial(upload_one_remote, client) + with ThreadPool(workers) as pool: + for mf, file_uuid in pool.imap_unordered(upload, pending): + uploaded[mf.host_path] = file_uuid + record_local_uuid(mf, file_uuid, store) + return uploaded def _build_payload( @@ -403,8 +441,12 @@ def _build_payload( ) -> dict[str, object]: """Build the JSON payload for POST /v1/instances.""" if args.shell: - command = " ".join(args.command_args) + # In shell mode the API runs `sh -c `, so we must + # rebuild the original argv into a shell-safe expression. + command = shlex.join(args.command_args) else: + # In non-shell mode the API exec's command + args directly, + # JSON list elements preserve boundaries, no quoting needed. parts = args.command_args command = parts[0] if parts else "" @@ -534,9 +576,7 @@ def cmd_run(args: RunArgs) -> int | None: print(str(exc), file=sys.stderr) return 1 - uploaded: dict[str, str] = {} - for mf in expanded_files: - uploaded[mf.host_path] = _upload_file(client, mf, store) + uploaded = upload_files(client, expanded_files, store) # 2b. Include pending files from session store pending = store.pending_files() @@ -670,6 +710,14 @@ def _norm(item: object) -> dict[str, object]: # 6. Cache terminal operation result store.cache[(op_uuid, "operation")] = op + if op["status"] != "SUCCESS": + logger.fatal( + "Operation %s ended with status %s%s", + op_uuid, + op["status"], + f": {op['error']}" if op.get("error") else "", + ) + # 7. Display result _display_operation(op, formatter) diff --git a/contree_cli/client.py b/contree_cli/client.py index 495588e..2a7e600 100644 --- a/contree_cli/client.py +++ b/contree_cli/client.py @@ -2,6 +2,7 @@ import base64 import http.client +import io import json import logging import platform @@ -10,6 +11,7 @@ from abc import ABC, abstractmethod from collections.abc import Iterator from importlib.metadata import PackageNotFoundError, version +from typing import IO, cast from urllib.parse import urlencode, urlsplit from contree_cli.config import AuthType, ConfigProfile @@ -33,6 +35,93 @@ def _cli_version() -> str: ) +class BodyFormatter: + """Lazy %s-arg for logging HTTP bodies — formats only on emit.""" + + def __init__( + self, + body: bytes | str | IO[bytes] | None, + content_type: str = "", + binary_max_size: int = 4096, + ) -> None: + self.body = body + self.binary_max_size = binary_max_size + self.content_type = content_type + + def __str__(self) -> str: + match self.body: + case None: + return self.format_none() + case bytes() | bytearray() as data: + return self.dispatch_bytes(bytes(data)) + case str() as text: + return self.dispatch_bytes(text.encode("utf-8", errors="replace")) + case _: + return self.format_stream() + + def format_none(self) -> str: + return "" + + def format_stream(self) -> str: + return "" + + def dispatch_bytes(self, data: bytes) -> str: + if not data: + return "" + if self.content_type and ( + "json" in self.content_type or "text" in self.content_type + ): + return self.format_json(data) + if self.content_type: + return self.format_bytes(data) + return self.format_json(data) + + def format_json(self, data: bytes) -> str: + truncated = data[: self.binary_max_size] + try: + text = truncated.decode("utf-8") + except UnicodeDecodeError: + return self.format_bytes(data) + if len(data) > self.binary_max_size: + return f"{text}... " + return text + + def format_bytes(self, data: bytes) -> str: + if self.content_type: + return f"" + return f"" + + +class BufferedResponse: + """Replay an HTTPResponse from buffered bytes (for debug body logging).""" + + def __init__( + self, + status: int, + reason: str, + headers: list[tuple[str, str]], + data: bytes, + ) -> None: + self.status = status + self.reason = reason + self.headers = headers + self.buf = io.BytesIO(data) + + def read(self, amt: int | None = None) -> bytes: + if amt is None: + return self.buf.read() + return self.buf.read(amt) + + def getheader(self, name: str, default: str | None = None) -> str | None: + for k, v in self.headers: + if k.lower() == name.lower(): + return v + return default + + def getheaders(self) -> list[tuple[str, str]]: + return list(self.headers) + + class ApiError(Exception): """Raised when the contree API returns a non-2xx status.""" @@ -80,7 +169,7 @@ def request( method: str, path: str, *, - body: bytes | None = None, + body: bytes | IO[bytes] | None = None, headers: dict[str, str] | None = None, ) -> http.client.HTTPResponse: merged: dict[str, str] = { @@ -96,6 +185,13 @@ def request( last_error: ApiError | None = None attempts = len(RETRY_DELAYS) + 1 + log.debug( + "%s %s body=%s", + method, + full_path, + BodyFormatter(body, content_type=merged.get("Content-Type", "")), + ) + for attempt in range(attempts): if last_error is not None: delay = RETRY_DELAYS[attempt - 1] @@ -106,12 +202,15 @@ def request( ) time.sleep(delay) - log.debug( - "%s %s body=%s", - method, - full_path, - f"{len(body)}B" if body is not None else "none", - ) + if attempt > 0 and hasattr(body, "seek"): + stream = cast(IO[bytes], body) + if not stream.seekable(): + raise ApiError( + 0, + "RetryNotSeekable", + "Cannot retry: streaming body is not seekable", + ) + stream.seek(0) try: conn = self._connect() conn.request(method, full_path, body, merged) @@ -127,6 +226,8 @@ def request( resp.status, resp.reason, ) + if log.isEnabledFor(logging.DEBUG): + return self.log_and_buffer(method, full_path, resp) return resp resp_body = resp.read().decode("utf-8", errors="replace") @@ -138,6 +239,15 @@ def request( resp.reason, len(resp_body), ) + log.debug( + "%s %s response body: %s", + method, + full_path, + BodyFormatter( + resp_body, + content_type=resp.getheader("Content-Type", "") or "", + ), + ) error = ApiError(resp.status, resp.reason, resp_body) if 500 <= resp.status < 600: @@ -149,6 +259,40 @@ def request( assert last_error is not None raise last_error + def log_and_buffer( + self, + method: str, + full_path: str, + resp: http.client.HTTPResponse, + ) -> http.client.HTTPResponse: + """Read & log a textual response body; pass binary streams through.""" + content_type = resp.getheader("Content-Type", "") or "" + textual = not content_type or "json" in content_type or "text" in content_type + if not textual: + log.debug( + "%s %s response body: ", + method, + full_path, + content_type, + ) + return resp + data = resp.read() + log.debug( + "%s %s response body: %s", + method, + full_path, + BodyFormatter(data, content_type=content_type), + ) + return cast( + http.client.HTTPResponse, + BufferedResponse( + status=resp.status, + reason=resp.reason, + headers=list(resp.getheaders()), + data=data, + ), + ) + # -- convenience methods -------------------------------------------------- def get( @@ -217,7 +361,7 @@ def _build_headers(self) -> dict[str, str]: class ContreeIAMClient(ContreeClient): """Client using IAM authentication with a project header.""" - DEFAULT_URL = "https://api.studio.nebius.com/sandboxes" + DEFAULT_URL = "https://api.tokenfactory.nebius.com/sandboxes" def __init__( self, diff --git a/contree_cli/config.py b/contree_cli/config.py index 4b133e5..24edfaf 100644 --- a/contree_cli/config.py +++ b/contree_cli/config.py @@ -1,6 +1,10 @@ +from __future__ import annotations + import configparser import logging import os +import shutil +import stat from collections.abc import Iterator, MutableMapping from dataclasses import dataclass from enum import Enum @@ -8,9 +12,26 @@ log = logging.getLogger(__name__) -CONTREE_HOME = Path(os.getenv("CONTREE_HOME", "~/.config/contree-cli")).expanduser() +CONTREE_HOME = Path(os.getenv("CONTREE_HOME", "~/.config/contree")).expanduser() CONFIG_DIR = CONTREE_HOME -CONFIG_FILE = CONTREE_HOME / "config.ini" +CONFIG_FILE = CONTREE_HOME / "auth.ini" +CLI_CONFIG_FILE = CONTREE_HOME / "cli.ini" + +# Parsed at import time. Paths are fixed by CONTREE_HOME (env), so the +# ``[cli]`` section is available before argparse runs and can supply +# defaults that beat hardcoded ones but still lose to flags. +SETTINGS = configparser.ConfigParser() +SETTINGS.read([CLI_CONFIG_FILE, CONFIG_FILE]) + +# Default editor for ``contree file edit`` when ``--editor`` is not given. +# Resolved once at import time. Priority: $EDITOR > cli.ini > vim > nano > vi. +EDITOR = ( + os.environ.get("EDITOR") + or SETTINGS.get("cli", "editor", fallback=None) + or shutil.which("vim") + or shutil.which("nano") + or "vi" +) class AuthType(str, Enum): @@ -39,7 +60,7 @@ def __repr__(self) -> str: @property def session_db_path(self) -> Path: - return CONTREE_HOME / f"sessions-{self.name}.db" + return CONTREE_HOME / "cli" / "sessions" / f"{self.name}.db" def remove_session_db(self) -> None: db = self.session_db_path @@ -55,10 +76,13 @@ class Config(MutableMapping[str, ConfigProfile]): ``del cfg[name]``, ``name in cfg``, ``len(cfg)``, iteration. """ - DEFAULT_IAM_URL = "https://api.studio.nebius.com/sandboxes" + DEFAULT_IAM_URL = "https://api.tokenfactory.nebius.com/sandboxes" PROFILE_PREFIX = "profile:" def __init__(self, path: Path | None = None) -> None: + from contree_cli.migrations import run_migrations + + run_migrations(CONTREE_HOME) self.__path = path or CONFIG_FILE self.__profiles: dict[str, ConfigProfile] = {} self.__active: str = "default" @@ -68,8 +92,12 @@ def __init__(self, path: Path | None = None) -> None: def _load(self) -> None: cp = configparser.ConfigParser() - log.debug("Loading config from %s", self.__path) - cp.read(self.__path) + # Read cli.ini first, then auth.ini — later files override earlier + # ones, so secrets in auth.ini take precedence over any non-secret + # profile fields a user keeps in cli.ini (url, project, type, …). + sources = [CLI_CONFIG_FILE, self.__path] + log.debug("Loading config from %s", sources) + cp.read(sources) self.__active = cp.defaults().get("profile", "default") self.__profiles.clear() for section in cp.sections(): @@ -106,8 +134,16 @@ def _save(self) -> None: if profile.project is not None: cp.set(section, "project", profile.project) self.__path.parent.mkdir(parents=True, exist_ok=True) - with open(self.__path, "w") as f: + # Create with 0o600 from the start so the token is never readable + # by other users — even between create() and chmod(). + fd = os.open( + self.__path, + os.O_WRONLY | os.O_CREAT | os.O_TRUNC, + stat.S_IRUSR | stat.S_IWUSR, + ) + with os.fdopen(fd, "w") as f: cp.write(f) + os.chmod(self.__path, stat.S_IRUSR | stat.S_IWUSR) # -- MutableMapping interface -------------------------------------------- diff --git a/contree_cli/migrations.py b/contree_cli/migrations.py new file mode 100644 index 0000000..95ee480 --- /dev/null +++ b/contree_cli/migrations.py @@ -0,0 +1,92 @@ +"""One-shot data layout migrations for ``CONTREE_HOME``. + +DELETE-ME: this entire module is transitional. It carries users of +``contree-cli <= 0.4.x`` (flat layout, ``~/.config/contree-cli/``) onto +the current layout (``~/.config/contree/`` with a ``cli/`` subdir). + +After ~2 minor releases (target: ``0.7.0``) drop this file, the +``run_migrations`` import and call in ``config.py``, and +``tests/test_migrations.py``. Anyone still on the old layout at that +point can do the rename by hand from the changelog. +""" + +from __future__ import annotations + +import logging +from pathlib import Path + +log = logging.getLogger(__name__) + +LEGACY_DIRNAME = "contree-cli" +LEGACY_CONFIG_BASENAME = "config.ini" + + +def run_migrations(home: Path) -> None: + """Idempotent layout migrations into ``home``.""" + migrate_legacy_dir(home) + migrate_into_cli_subdir(home) + + +def migrate_legacy_dir(home: Path) -> None: + """Move ``/contree-cli/*`` into ``home``. + + Renames ``config.ini`` to ``auth.ini``. Skipped when ``home/auth.ini`` + already exists, so freshly-created session DBs in ``home`` don't + block credential migration. + """ + legacy_dir = home.parent / LEGACY_DIRNAME + if not legacy_dir.is_dir(): + return + if (home / "auth.ini").exists(): + return + + log.info("Migrating CONTREE_HOME: %s -> %s", legacy_dir, home) + home.mkdir(parents=True, exist_ok=True) + + for item in legacy_dir.iterdir(): + target_name = "auth.ini" if item.name == LEGACY_CONFIG_BASENAME else item.name + target = home / target_name + if target.exists(): + log.warning("Skipping %s: %s already exists", item, target) + continue + item.rename(target) + + try: + legacy_dir.rmdir() + except OSError: + log.debug( + "Legacy dir %s not empty after migration; leaving in place", legacy_dir + ) + + +def migrate_into_cli_subdir(home: Path) -> None: + """Move flat session/skill DBs into the ``cli/`` subdirectory. + + - ``home/sessions-{name}.db*`` → ``home/cli/sessions/{name}.db*`` + - ``home/skills.db*`` → ``home/cli/skills.db*`` + """ + if not home.is_dir(): + return + cli_dir = home / "cli" + sessions_dir = cli_dir / "sessions" + moved_any = False + + for item in home.iterdir(): + if not item.is_file(): + continue + if item.name.startswith("sessions-"): + target = sessions_dir / item.name[len("sessions-") :] + sessions_dir.mkdir(parents=True, exist_ok=True) + elif item.name.startswith("skills.db"): + target = cli_dir / item.name + cli_dir.mkdir(parents=True, exist_ok=True) + else: + continue + + if target.exists(): + log.warning("Skipping %s: %s already exists", item, target) + continue + if not moved_any: + log.info("Reorganising CLI state into %s", cli_dir) + moved_any = True + item.rename(target) diff --git a/contree_cli/session.py b/contree_cli/session.py index 1e633df..f98c6c2 100644 --- a/contree_cli/session.py +++ b/contree_cli/session.py @@ -13,6 +13,8 @@ from functools import cached_property from pathlib import Path, PurePosixPath +CONTREE_CONCURRENCY = int(os.getenv("CONTREE_CONCURRENCY", "8")) + @dataclass(frozen=True) class Session: diff --git a/contree_cli/shell/repl.py b/contree_cli/shell/repl.py index b560b2a..51e0a3e 100644 --- a/contree_cli/shell/repl.py +++ b/contree_cli/shell/repl.py @@ -4,7 +4,6 @@ import contextlib import logging -import os import re import shlex import sys @@ -391,15 +390,7 @@ def dispatch_edit(self, editor: str, args: list[str]) -> None: print(f"Usage: {editor} ", file=sys.stderr) return resolved = self.resolve_path(args[0]) - old_editor = os.environ.get("EDITOR") - os.environ["EDITOR"] = editor - try: - self.dispatch_contree(["file", "edit", resolved]) - finally: - if old_editor is None: - os.environ.pop("EDITOR", None) - else: - os.environ["EDITOR"] = old_editor + self.dispatch_contree(["file", "edit", "--editor", editor, resolved]) GLOB_CHARS = frozenset("*?[") diff --git a/contree_cli/skill.py b/contree_cli/skill.py index ea87268..4fe7f31 100644 --- a/contree_cli/skill.py +++ b/contree_cli/skill.py @@ -64,7 +64,7 @@ def default_claude_home() -> Path: @contextmanager def connect_registry() -> Iterator[sqlite3.Connection]: - db_path = config_mod.CONTREE_HOME / "skills.db" + db_path = config_mod.CONTREE_HOME / "cli" / "skills.db" db_path.parent.mkdir(parents=True, exist_ok=True) conn = sqlite3.connect(str(db_path), timeout=5.0) conn.row_factory = sqlite3.Row diff --git a/docs/commands/auth.md b/docs/commands/auth.md index 79f33ab..8ba6306 100644 --- a/docs/commands/auth.md +++ b/docs/commands/auth.md @@ -45,14 +45,14 @@ When you run `contree auth`, the CLI: shell history) 2. Prompts for the **project ID** 3. Verifies the token with the API (`GET /v1/whoami`) -4. Writes credentials to `~/.config/contree-cli/config.ini` +4. Writes credentials to `~/.config/contree/auth.ini` 5. If the profile already exists, prompts for confirmation (use `-y` to skip) ### Flags - `--token` — API token (prompted securely if omitted) -- `--url` — API base URL (default: `https://api.studio.nebius.com/sandboxes`) +- `--url` — API base URL (default: `https://api.tokenfactory.nebius.com/sandboxes`) - `--project` — Project ID (prompted if omitted) - `--profile` — Profile name (default: `default`) - `-y` / `--force` — Overwrite existing profile without confirmation diff --git a/docs/tutorial/configuration.md b/docs/tutorial/configuration.md index 6d5664c..490793f 100644 --- a/docs/tutorial/configuration.md +++ b/docs/tutorial/configuration.md @@ -16,7 +16,7 @@ contree auth --profile=sandbox ``` Each command prompts for a token securely (no echo), verifies it against -the API, and writes it to `~/.config/contree-cli/config.ini`. +the API, and writes it to `~/.config/contree/auth.ini`. The resulting config file looks like this: @@ -26,19 +26,19 @@ profile = default [profile:default] token = eyJ... -url = https://api.studio.nebius.com/sandboxes +url = https://api.tokenfactory.nebius.com/sandboxes type = iam project = project-id-default [profile:personal] token = eyJ... -url = https://api.studio.nebius.com/sandboxes +url = https://api.tokenfactory.nebius.com/sandboxes type = iam project = project-id-personal [profile:sandbox] token = eyJ... -url = https://api.studio.nebius.com/sandboxes +url = https://api.tokenfactory.nebius.com/sandboxes type = iam project = project-id-sandbox ``` @@ -111,7 +111,7 @@ contree images # uses sandbox Pass `--token` and `--url` directly: ```bash -contree --token=eyJ... --url=https://api.studio.nebius.com/sandboxes images +contree --token=eyJ... --url=https://api.tokenfactory.nebius.com/sandboxes images ``` :::{warning} @@ -134,7 +134,7 @@ remaining profile. ## Profiles and sessions Each profile has its own session database -(`~/.config/contree-cli/sessions-{profile}.db`), so: +(`~/.config/contree/sessions-{profile}.db`), so: - **Same profile, same terminal** — resumes the existing session - **Different profile, same terminal** — different session, different data @@ -152,13 +152,14 @@ export CONTREE_SESSION=shared-session ## Data storage -All data lives in `CONTREE_HOME` (default `~/.config/contree-cli`): +All data lives in `CONTREE_HOME` (default `~/.config/contree`): -| File | Purpose | +| Path | Purpose | |------|---------| -| `config.ini` | Profile credentials and settings | -| `sessions-{profile}.db` | Per-profile sessions, history, branches, cache | -| `skills.db` | Installed agent skill registry | +| `auth.ini` | Profile credentials and settings (created with mode `0600`) | +| `cli.ini` | Optional user-editable defaults for the CLI | +| `cli/sessions/{profile}.db` | Per-profile sessions, history, branches, cache | +| `cli/skills.db` | Installed agent skill registry | Override with `$CONTREE_HOME`: @@ -166,11 +167,82 @@ Override with `$CONTREE_HOME`: export CONTREE_HOME=/custom/path ``` +### `cli.ini` + +`cli.ini` is meant for hand-editing. Create it yourself; the CLI never +writes to it. Two kinds of sections are supported: + +#### `[cli]` section: per-flag defaults + +Keys here become argparse defaults. Use the argparse `dest` name (not +the flag name): + +| Key | Maps to flag | Notes | +|-----|--------------|-------| +| `log_level` | `--log-level` | One of `debug`, `info`, `warning`, `error`, `critical` | +| `output_format` | `-f` / `--format` | One of the formatter names (`default`, `json`, `json-pretty`, `csv`, `tsv`, `table`) | +| `editor` | `--editor` (file edit) | Fallback when neither `--editor` nor `$EDITOR` is set; if absent the CLI searches `vim` then `nano` on `PATH` and falls back to `vi` | + +Example: + +```ini +[cli] +log_level = debug +output_format = json +editor = nvim +``` + +Precedence: CLI flag > environment variable > `cli.ini` > built-in +default. A `cli.ini` setting always loses to an explicit flag. + +#### `[profile:NAME]` sections: CLI-scoped profiles + +`cli.ini` accepts the same `[profile:NAME]` sections as `auth.ini` and +supports the same fields: + +| Key | Required | Notes | +|-----|----------|-------| +| `url` | yes for JWT, optional for IAM | API base URL | +| `type` | optional | `jwt` (default) or `iam` | +| `project` | IAM only | Project ID | +| `token` | optional | API bearer token | + +The two files are merged at load time and `auth.ini` wins on conflict. + +What `cli.ini` is for: profiles (or any field, including `token`) you +want only the `contree` CLI to see. The CLI merges `cli.ini` with +`auth.ini`. Other contree-related tooling that talks to the API +directly (the SDK, the MCP server) reads only `auth.ini`. Use +`cli.ini` when you need a profile that should be invisible to those +direct-API consumers, or to keep `auth.ini` minimal and shared. + +Example, CLI-only profile alongside the shared one: + +```ini +# ~/.config/contree/auth.ini (read by CLI + SDK + MCP, mode 0600) +[DEFAULT] +profile = default + +[profile:default] +url = https://contree.dev +token = eyJhbGciOi... +``` + +```ini +# ~/.config/contree/cli.ini (read only by the CLI) +[profile:cli-sandbox] +url = https://staging.contree.dev +token = eyJhbGciOi...different +``` + +The active profile is still selected by the `profile` key in +`[DEFAULT]` of `auth.ini` (or by `--profile` / `$CONTREE_PROFILE`). + ## Environment variables | Variable | Description | |----------|-------------| -| `CONTREE_HOME` | Data directory (default `~/.config/contree-cli`) | +| `CONTREE_HOME` | Data directory (default `~/.config/contree`) | | `CONTREE_TOKEN` | API bearer token (overrides config) | | `CONTREE_URL` | API base URL (overrides config) | | `CONTREE_PROJECT` | Project ID (overrides config) | @@ -184,7 +256,7 @@ For token, URL, and project: 1. CLI flag (`--token`, `--url`, `--project`) 2. Environment variable (`CONTREE_TOKEN`, `CONTREE_URL`, `CONTREE_PROJECT`) 3. Config file value from the active profile -4. Built-in default URL: `https://api.studio.nebius.com/sandboxes` +4. Built-in default URL: `https://api.tokenfactory.nebius.com/sandboxes` For profiles: diff --git a/docs/tutorial/installation.md b/docs/tutorial/installation.md index 286edde..d879245 100644 --- a/docs/tutorial/installation.md +++ b/docs/tutorial/installation.md @@ -71,7 +71,7 @@ You will be prompted to enter: 2. **Project ID** — your project identifier The CLI verifies the token with the API and writes credentials to -`~/.config/contree-cli/config.ini`. If a profile already exists you will be +`~/.config/contree/auth.ini`. If a profile already exists you will be prompted to confirm; use `-y` to skip the prompt. Resolution order for each field (first match wins): diff --git a/docs/tutorial/sessions.md b/docs/tutorial/sessions.md index 0965518..d6a3697 100644 --- a/docs/tutorial/sessions.md +++ b/docs/tutorial/sessions.md @@ -208,12 +208,12 @@ and does not require manual session key export. ## Storage -Session data is stored in a SQLite database at -`~/.local/lib/contree-cli/sessions.db`. Override with -`CONTREE_SESSION_DB`: +Session data is stored in a per-profile SQLite database at +`~/.config/contree/sessions-{profile}.db`. Override the data +directory with `CONTREE_HOME`: ```bash -export CONTREE_SESSION_DB=/tmp/my-sessions.db +export CONTREE_HOME=/tmp/contree-data ``` --- diff --git a/tests/conftest.py b/tests/conftest.py index 7a0a105..4e3854f 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -35,17 +35,35 @@ class FakeResponse: status: int = 200 reason: str = "" body: bytes = b"{}" + headers: dict[str, str] | None = None def __post_init__(self) -> None: if not self.reason: self.reason = "OK" if self.status < 300 else "Error" + if self.headers is None: + self.headers = {} def read(self, amt: int | None = None) -> bytes: return self.body + def getheader(self, name: str, default: str | None = None) -> str | None: + assert self.headers is not None + for key, value in self.headers.items(): + if key.lower() == name.lower(): + return value + return default + + def getheaders(self) -> list[tuple[str, str]]: + assert self.headers is not None + return list(self.headers.items()) + @staticmethod def json(body: object, *, status: int = 200) -> FakeResponse: - return FakeResponse(status=status, body=json.dumps(body).encode()) + return FakeResponse( + status=status, + body=json.dumps(body).encode(), + headers={"Content-Type": "application/json"}, + ) @dataclass @@ -169,8 +187,8 @@ def iam_client() -> ContreeTestIAMClient: def config_dir(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> Path: """Redirect CONTREE_HOME / CONFIG_DIR / CONFIG_FILE to a temp directory.""" home = tmp_path / ".contree" - cfg_dir = home / "contree-cli" - cfg_file = cfg_dir / "config.ini" + cfg_dir = home / "contree" + cfg_file = cfg_dir / "auth.ini" monkeypatch.setattr(config_mod, "CONTREE_HOME", home) monkeypatch.setattr(config_mod, "CONFIG_DIR", cfg_dir) monkeypatch.setattr(config_mod, "CONFIG_FILE", cfg_file) diff --git a/tests/test_client.py b/tests/test_client.py index 0b6f5cc..30ef44e 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -1,6 +1,7 @@ from __future__ import annotations import json +import logging from unittest.mock import patch import pytest @@ -10,6 +11,7 @@ CLI_USER_AGENT, RETRY_DELAYS, ApiError, + BodyFormatter, ContreeClient, ContreeJWTClient, resolve_image, @@ -296,11 +298,168 @@ def test_cannot_instantiate_directly(self): ContreeClient("https://example.com", "tok") # type: ignore[abstract] +# --------------------------------------------------------------------------- +# Streaming body +# --------------------------------------------------------------------------- + + +class TestStreamingBody: + def test_passes_file_object_to_connection(self): + import io + + c = ContreeTestClient("https://contree.dev", "tok") + c.respond(status=201, body=b'{"uuid":"u1"}') + stream = io.BytesIO(b"a" * 1024) + c.request( + "POST", + "/v1/files", + body=stream, + headers={"Content-Type": "application/octet-stream"}, + ) + sent = c.get_request(-1).body + assert sent is stream + + def test_retry_seeks_back_to_start(self): + import io + + c = ContreeTestClient("https://contree.dev", "tok") + c.respond(status=503, body=b"down") + c.respond(status=201, body=b'{"uuid":"u1"}') + + seeks: list[int] = [] + + class TrackingStream(io.BytesIO): + def seek(self, pos, whence=0): # type: ignore[override] + seeks.append(pos) + return super().seek(pos, whence) + + def seekable(self) -> bool: # type: ignore[override] + return True + + stream = TrackingStream(b"payload") + + with patch("contree_cli.client.time.sleep"): + c.request("POST", "/v1/files", body=stream) + + assert seeks == [0] + + def test_retry_unseekable_stream_raises(self): + import io + + c = ContreeTestClient("https://contree.dev", "tok") + c.respond(status=500, body=b"down") + + class Unseekable(io.BytesIO): + def seekable(self) -> bool: # type: ignore[override] + return False + + with patch("contree_cli.client.time.sleep"), pytest.raises(ApiError) as ei: + c.request("POST", "/v1/files", body=Unseekable(b"x")) + + assert ei.value.reason == "RetryNotSeekable" + + # --------------------------------------------------------------------------- # IAM client # --------------------------------------------------------------------------- +class TestDebugLogging: + def _enable_debug(self, caplog: pytest.LogCaptureFixture) -> None: + caplog.set_level(logging.DEBUG, logger="contree_cli.client") + + def test_logs_request_body_when_debug(self, caplog): + self._enable_debug(caplog) + c = ContreeTestClient("https://contree.dev", "tok") + c.respond(status=201, body=b'{"uuid":"x"}') + c.post_json("/v1/instances", {"image": "ubuntu", "command": "uname -a"}) + msgs = "\n".join(r.getMessage() for r in caplog.records) + assert '"image": "ubuntu"' in msgs + assert '"command": "uname -a"' in msgs + + def test_logs_error_response_body_when_debug(self, caplog): + self._enable_debug(caplog) + c = ContreeTestClient("https://contree.dev", "tok") + c.respond(status=400, body=b'{"error":"bad payload"}') + with pytest.raises(ApiError): + c.post_json("/v1/instances", {"image": "ubuntu"}) + msgs = "\n".join(r.getMessage() for r in caplog.records) + assert '"error":"bad payload"' in msgs + + def test_no_body_logs_when_not_debug(self, caplog): + caplog.set_level(logging.INFO, logger="contree_cli.client") + c = ContreeTestClient("https://contree.dev", "tok") + c.respond(status=200, body=b'{"a":1}') + c.post_json("/v1/instances", {"x": 1}) + msgs = "\n".join(r.getMessage() for r in caplog.records) + assert "request body" not in msgs + + def test_octet_stream_request_body_not_dumped(self, caplog): + self._enable_debug(caplog) + c = ContreeTestClient("https://contree.dev", "tok") + c.respond(status=201, body=b"{}") + binary = bytes(range(256)) + c.request( + "POST", + "/v1/files", + body=binary, + headers={"Content-Type": "application/octet-stream"}, + ) + msgs = "\n".join(r.getMessage() for r in caplog.records) + assert "" + + def test_empty(self): + assert str(BodyFormatter(b"")) == "" + + def test_text_body(self): + assert str(BodyFormatter(b'{"hi":1}')) == '{"hi":1}' + + def test_truncation(self): + body = b"a" * 5000 + out = str(BodyFormatter(body, binary_max_size=100)) + assert out.startswith("a" * 100) + assert "5000B total" in out + + def test_binary_content_type(self): + out = str( + BodyFormatter( + b"\xff\xfe\x00", + content_type="application/octet-stream", + ) + ) + assert "" + + def test_lazy_str_only_called_at_format(self): + """BodyFormatter must defer its work until __str__ is invoked.""" + calls: list[int] = [] + + class Counting(BodyFormatter): + def __str__(self) -> str: + calls.append(1) + return super().__str__() + + # Logging at a level above DEBUG must not call __str__. + logger = logging.getLogger("contree_cli.client") + prev = logger.level + logger.setLevel(logging.WARNING) + try: + logger.debug("body=%s", Counting(b"hello")) + finally: + logger.setLevel(prev) + assert calls == [] + + class TestContreeIAMClient: def test_injects_project_header(self): c = ContreeTestIAMClient("https://example.com", "tok", "aiproject-test") diff --git a/tests/test_config.py b/tests/test_config.py index 85f1fe6..a89a79e 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -1,8 +1,15 @@ import configparser +import os +import stat +import sys import pytest -from contree_cli.config import AuthType, Config, ConfigProfile +from contree_cli.config import ( + AuthType, + Config, + ConfigProfile, +) # --------------------------------------------------------------------------- # save / load via Config @@ -17,7 +24,7 @@ def test_save_creates_file(self, config_dir): token="tok123", url="https://test.dev", ) - assert (config_dir / "config.ini").exists() + assert (config_dir / "auth.ini").exists() def test_load_reads_saved_profile(self, config_dir): cfg = Config() @@ -171,9 +178,9 @@ def test_url_falls_back_for_jwt_when_missing(self, config_dir): url="https://test.dev", ) cp = configparser.ConfigParser() - cp.read(config_dir / "config.ini") + cp.read(config_dir / "auth.ini") cp.remove_option(Config.PROFILE_PREFIX + "default", "url") - with open(config_dir / "config.ini", "w") as f: + with open(config_dir / "auth.ini", "w") as f: cp.write(f) p = Config().resolve() assert p.url == "" @@ -188,9 +195,9 @@ def test_url_falls_back_for_iam_when_missing(self, config_dir): auth_type=AuthType.IAM, ) cp = configparser.ConfigParser() - cp.read(config_dir / "config.ini") + cp.read(config_dir / "auth.ini") cp.remove_option(Config.PROFILE_PREFIX + "default", "url") - with open(config_dir / "config.ini", "w") as f: + with open(config_dir / "auth.ini", "w") as f: cp.write(f) p = Config().resolve() assert p.url == Config.DEFAULT_IAM_URL @@ -242,9 +249,9 @@ def test_legacy_profile_without_type_is_jwt(self, config_dir): url="https://old.dev", ) cp = configparser.ConfigParser() - cp.read(config_dir / "config.ini") + cp.read(config_dir / "auth.ini") cp.remove_option(Config.PROFILE_PREFIX + "default", "type") - with open(config_dir / "config.ini", "w") as f: + with open(config_dir / "auth.ini", "w") as f: cp.write(f) p = Config().resolve() assert p.auth_type == AuthType.JWT @@ -352,3 +359,85 @@ def test_switch_nonexistent_raises(self, config_dir): cfg = Config() with pytest.raises(ValueError, match="does not exist"): cfg.switch("nonexistent") + + +# --------------------------------------------------------------------------- +# auth.ini permissions +# --------------------------------------------------------------------------- + + +@pytest.mark.skipif(sys.platform == "win32", reason="POSIX permissions") +class TestAuthFilePermissions: + def test_file_mode_is_0600(self, config_dir): + cfg = Config() + cfg["default"] = ConfigProfile( + name="default", + token="secret-token", + url="https://test.dev", + ) + path = config_dir / "auth.ini" + mode = stat.S_IMODE(os.stat(path).st_mode) + assert mode == 0o600 + + def test_rewrite_keeps_0600(self, config_dir): + cfg = Config() + cfg["default"] = ConfigProfile( + name="default", + token="t1", + url="https://test.dev", + ) + path = config_dir / "auth.ini" + os.chmod(path, 0o644) + cfg["default"] = ConfigProfile( + name="default", + token="t2", + url="https://test.dev", + ) + mode = stat.S_IMODE(os.stat(path).st_mode) + assert mode == 0o600 + + +# --------------------------------------------------------------------------- +# auth.ini + cli.ini are read together; auth.ini wins on conflict +# --------------------------------------------------------------------------- + + +class TestProfileMergeAcrossFiles: + def test_profile_fields_merge_from_cli_and_auth(self, config_dir, monkeypatch): + import contree_cli.config as config_mod + + cli_path = config_dir / "cli.ini" + cli_path.parent.mkdir(parents=True, exist_ok=True) + cli_path.write_text( + "[profile:default]\nurl = https://from-cli.dev\nproject = aiproject-cli\n" + ) + monkeypatch.setattr(config_mod, "CLI_CONFIG_FILE", cli_path) + + auth_path = config_dir / "auth.ini" + auth_path.write_text( + "[DEFAULT]\nprofile = default\n[profile:default]\ntoken = secret-tok\n" + ) + + p = Config().resolve() + assert p.token == "secret-tok" + assert p.url == "https://from-cli.dev" + assert p.project == "aiproject-cli" + + def test_auth_overrides_cli_on_conflict(self, config_dir, monkeypatch): + import contree_cli.config as config_mod + + cli_path = config_dir / "cli.ini" + cli_path.parent.mkdir(parents=True, exist_ok=True) + cli_path.write_text("[profile:default]\nurl = https://from-cli.dev\n") + monkeypatch.setattr(config_mod, "CLI_CONFIG_FILE", cli_path) + + auth_path = config_dir / "auth.ini" + auth_path.write_text( + "[DEFAULT]\nprofile = default\n" + "[profile:default]\n" + "url = https://from-auth.dev\n" + "token = tok\n" + ) + + p = Config().resolve() + assert p.url == "https://from-auth.dev" diff --git a/tests/test_file_cmd.py b/tests/test_file_cmd.py index d1aa87b..332e152 100644 --- a/tests/test_file_cmd.py +++ b/tests/test_file_cmd.py @@ -3,6 +3,7 @@ import io import json from contextvars import copy_context +from dataclasses import replace from pathlib import Path from unittest.mock import patch @@ -32,6 +33,9 @@ def __init__(self, data: bytes, *, status: int = 200): def read(self, amt: int | None = None) -> bytes: return self._buf.read(amt) + def getheader(self, name: str, default: str | None = None) -> str | None: + return default + def _api_response(body: bytes | dict, *, status: int = 200) -> StreamResponse: data = json.dumps(body).encode() if isinstance(body, dict) else body @@ -46,23 +50,43 @@ def _run_file_edit( store: SessionStore, editor_content: bytes | None = None, ) -> tuple[int | None]: + """Drive ``cmd_file_edit`` with mocked editor + HTTP responses. + + The editor mock writes ``editor_content`` directly to the file + inside the temp dir created by ``cmd_file_edit``. This sidesteps + shell-string parsing, which has subtle differences on Windows + when paths contain backslashes. + """ + import tempfile + tc.fake.responses.extend(responses) + if not args.editor: + args = replace(args, editor="fake-editor") + SESSION_STORE.set(store) ctx = copy_context() - def fake_editor(cmd: str, *, shell: bool = True) -> int: - # cmd is a shell string like "fake-editor '/tmp/...'" - import shlex + captured_dir: list[str] = [] + real_mkdtemp = tempfile.mkdtemp + + def capturing_mkdtemp(*a, **kw): + d = real_mkdtemp(*a, **kw) + captured_dir.append(d) + return d - parts = shlex.split(cmd) - if editor_content is not None: - Path(parts[1]).write_bytes(editor_content) + def fake_editor(cmd: str, *, shell: bool = True) -> int: + if editor_content is not None and captured_dir: + for f in Path(captured_dir[-1]).iterdir(): + f.write_bytes(editor_content) return 0 with ( + patch( + "contree_cli.cli.file.tempfile.mkdtemp", + side_effect=capturing_mkdtemp, + ), patch("contree_cli.cli.file.subprocess.call", side_effect=fake_editor), - patch.dict("os.environ", {"EDITOR": "fake-editor"}), ): rc = ctx.run(cmd_file_edit, args) return rc @@ -194,14 +218,8 @@ def test_editor_nonzero_exit(self, contree_client, session_store: SessionStore): SESSION_STORE.set(session_store) ctx = copy_context() - with ( - patch( - "contree_cli.cli.file.subprocess.call", - return_value=1, - ), - patch.dict("os.environ", {"EDITOR": "fake-editor"}), - ): - rc = ctx.run(cmd_file_edit, args) + with patch("contree_cli.cli.file.subprocess.call", return_value=1): + rc = ctx.run(cmd_file_edit, replace(args, editor="fake-editor")) assert rc == 1 assert session_store.pending_files() == [] @@ -233,10 +251,7 @@ def fake_editor(cmd: str, *, shell: bool = True) -> int: SESSION_STORE.set(session_store) ctx = copy_context() - with ( - patch("contree_cli.cli.file.subprocess.call", side_effect=fake_editor), - patch.dict("os.environ", {"EDITOR": "should-not-use"}), - ): + with patch("contree_cli.cli.file.subprocess.call", side_effect=fake_editor): rc = ctx.run(cmd_file_edit, args) assert rc is None assert called_with[0].startswith("nvim ") diff --git a/tests/test_migrations.py b/tests/test_migrations.py new file mode 100644 index 0000000..c873636 --- /dev/null +++ b/tests/test_migrations.py @@ -0,0 +1,99 @@ +"""Tests for the transitional ``contree_cli.migrations`` module. + +DELETE-ME together with ``contree_cli/migrations.py`` (target: 0.7.0). +""" + +from __future__ import annotations + +from contree_cli.migrations import run_migrations + + +class TestRunMigrations: + def test_renames_legacy_dir_and_reorganises(self, tmp_path): + legacy = tmp_path / "contree-cli" + legacy.mkdir() + (legacy / "config.ini").write_text("[DEFAULT]\nprofile = default\n") + (legacy / "sessions-default.db").write_bytes(b"sqlite-bytes") + (legacy / "sessions-default.db-wal").write_bytes(b"wal") + (legacy / "skills.db").write_bytes(b"skills-bytes") + + home = tmp_path / "contree" + run_migrations(home) + + assert not legacy.exists() + assert (home / "auth.ini").exists() + assert not (home / "config.ini").exists() + assert ( + home / "cli" / "sessions" / "default.db" + ).read_bytes() == b"sqlite-bytes" + assert (home / "cli" / "sessions" / "default.db-wal").exists() + assert (home / "cli" / "skills.db").read_bytes() == b"skills-bytes" + assert not (home / "sessions-default.db").exists() + assert not (home / "skills.db").exists() + + def test_no_op_when_legacy_missing(self, tmp_path): + home = tmp_path / "contree" + run_migrations(home) + assert not home.exists() + + def test_no_op_when_new_auth_already_exists(self, tmp_path): + legacy = tmp_path / "contree-cli" + legacy.mkdir() + (legacy / "config.ini").write_text("legacy") + + home = tmp_path / "contree" + home.mkdir() + (home / "auth.ini").write_text("already-there") + + run_migrations(home) + + assert legacy.exists() + assert (home / "auth.ini").read_text() == "already-there" + + def test_migrates_credentials_with_pre_existing_flat_session(self, tmp_path): + """A pre-existing sessions-*.db in home (e.g. auto-created by an + earlier broken release) must NOT block credential migration.""" + legacy = tmp_path / "contree-cli" + legacy.mkdir() + (legacy / "config.ini").write_text("[DEFAULT]\nprofile = default\n") + + home = tmp_path / "contree" + home.mkdir() + (home / "sessions-default.db").write_bytes(b"fresh-empty-db") + + run_migrations(home) + + assert (home / "auth.ini").exists() + assert ( + home / "cli" / "sessions" / "default.db" + ).read_bytes() == b"fresh-empty-db" + assert not (home / "sessions-default.db").exists() + + def test_flat_to_nested_runs_without_legacy(self, tmp_path): + """No legacy dir, but flat layout in home → reorganise only.""" + home = tmp_path / "contree" + home.mkdir() + (home / "auth.ini").write_text("[DEFAULT]\nprofile = default\n") + (home / "sessions-default.db").write_bytes(b"db") + (home / "skills.db").write_bytes(b"skills") + + run_migrations(home) + + assert (home / "cli" / "sessions" / "default.db").exists() + assert (home / "cli" / "skills.db").exists() + assert not (home / "sessions-default.db").exists() + assert not (home / "skills.db").exists() + + def test_skips_target_collision(self, tmp_path): + """If destination already has a file with the same name, keep the + source untouched rather than clobbering.""" + home = tmp_path / "contree" + home.mkdir() + (home / "cli").mkdir() + (home / "cli" / "skills.db").write_bytes(b"new-skills") + (home / "skills.db").write_bytes(b"old-skills") + + run_migrations(home) + + assert (home / "skills.db").read_bytes() == b"old-skills" + assert (home / "cli" / "skills.db").read_bytes() == b"new-skills" diff --git a/tests/test_run.py b/tests/test_run.py index 66a09f4..7e5d3cb 100644 --- a/tests/test_run.py +++ b/tests/test_run.py @@ -464,7 +464,8 @@ def test_file_dedup_logs_reuse( ] with caplog.at_level(logging.INFO): _run_cmd(contree_client, args, responses, store=session_store) - assert "already uploaded" in caplog.text + assert "Uploaded file:" in caplog.text + assert "existing-uuid" in caplog.text def test_file_dedup_non_404_raises(self, contree_client, session_store, tmp_path): """Non-404 error from GET /v1/files propagates.""" @@ -560,6 +561,70 @@ def test_local_file_cache_invalidated_when_file_changes( assert spawn_body["files"]["/app/cached-change.txt"]["uuid"] == "new-uuid" +class TestParallelUpload: + def test_upload_files_parallel_aggregates( + self, contree_client, session_store, tmp_path, monkeypatch + ): + """upload_files dispatches via ThreadPool and collects all uuids.""" + from contree_cli.cli import run as run_mod + + files = [] + for i in range(5): + p = tmp_path / f"f{i}.txt" + p.write_text(f"content-{i}") + files.append( + MappedFile( + host_path=str(p), + instance_path=f"/app/f{i}.txt", + uid=0, + gid=0, + mode=0o644, + ) + ) + + def fake_remote(client, mf): + return mf, f"uuid-for-{os.path.basename(mf.host_path)}" + + monkeypatch.setattr(run_mod, "upload_one_remote", fake_remote) + + result = run_mod.upload_files(contree_client, files, session_store) + + assert {mf.host_path for mf in files} == set(result.keys()) + for mf in files: + assert result[mf.host_path] == ( + f"uuid-for-{os.path.basename(mf.host_path)}" + ) + + def test_upload_files_skips_cached( + self, contree_client, session_store, tmp_path, monkeypatch + ): + """Files already in the local cache must not hit the upload pool.""" + from contree_cli.cli import run as run_mod + + p = tmp_path / "cached.txt" + p.write_text("data") + mf = MappedFile( + host_path=str(p), + instance_path="/app/cached.txt", + uid=0, + gid=0, + mode=0o644, + ) + run_mod.record_local_uuid(mf, "cached-uuid", session_store) + + called = [] + + def fake_remote(client, mfx): + called.append(mfx.host_path) + return mfx, "should-not-be-used" + + monkeypatch.setattr(run_mod, "upload_one_remote", fake_remote) + + result = run_mod.upload_files(contree_client, [mf], session_store) + assert result == {mf.host_path: "cached-uuid"} + assert called == [] + + # ── Spawn payload ──────────────────────────────────────────────────────── @@ -871,6 +936,67 @@ def test_non_shell_splits_command(self, contree_client, session_store): assert body["args"] == ["hello", "world"] assert body["shell"] is False + def test_non_shell_passes_args_raw(self, contree_client, session_store): + """Non-shell mode: command + args go to direct exec, no shell quoting. + + The API exec's argv directly, so adding shell quotes would put + literal quote characters into the program's argv. + """ + session_store.set_image(IMG_UUID, kind="test") + args = _default_args( + command_args=["python3", "-c", "print('hello world')"], + shell=False, + ) + responses = [ + _spawn_response(), + _op_response(status="SUCCESS", exit_code=0), + ] + _run_cmd(contree_client, args, responses, store=session_store) + spawn_req = contree_client.get_request(0) + body = json.loads(spawn_req.body) + assert body["command"] == "python3" + assert body["args"] == ["-c", "print('hello world')"] + + def test_shell_quotes_arg_with_spaces(self, contree_client, session_store): + session_store.set_image(IMG_UUID, kind="test") + args = _default_args( + command_args=["python3", "-c", "print('hello world')"], + shell=True, + ) + responses = [ + _spawn_response(), + _op_response(status="SUCCESS", exit_code=0), + ] + _run_cmd(contree_client, args, responses, store=session_store) + spawn_req = contree_client.get_request(0) + body = json.loads(spawn_req.body) + # shlex.join must round-trip back through shlex.split to the + # original argv when the remote shell parses the command. + import shlex + + assert shlex.split(body["command"]) == [ + "python3", + "-c", + "print('hello world')", + ] + + def test_shell_does_not_overquote_simple_tokens( + self, contree_client, session_store + ): + session_store.set_image(IMG_UUID, kind="test") + args = _default_args( + command_args=["ls", "-la", "/etc"], + shell=True, + ) + responses = [ + _spawn_response(), + _op_response(status="SUCCESS", exit_code=0), + ] + _run_cmd(contree_client, args, responses, store=session_store) + spawn_req = contree_client.get_request(0) + body = json.loads(spawn_req.body) + assert body["command"] == "ls -la /etc" + # ── Session update on success ──────────────────────────────────────────── diff --git a/tests/test_session.py b/tests/test_session.py index 4c4f123..b98698f 100644 --- a/tests/test_session.py +++ b/tests/test_session.py @@ -427,11 +427,14 @@ def test_without_env_var(self): class TestGetDbPath: def test_default_profile(self): p = ConfigProfile(name="default", url="", token=None) - assert p.session_db_path.name == "sessions-default.db" + assert p.session_db_path.name == "default.db" + assert p.session_db_path.parent.name == "sessions" + assert p.session_db_path.parent.parent.name == "cli" def test_named_profile(self): p = ConfigProfile(name="staging", url="", token=None) - assert p.session_db_path.name == "sessions-staging.db" + assert p.session_db_path.name == "staging.db" + assert p.session_db_path.parent.name == "sessions" class TestPendingFiles: diff --git a/tests/test_shell_repl.py b/tests/test_shell_repl.py index de8d33d..3d06acb 100644 --- a/tests/test_shell_repl.py +++ b/tests/test_shell_repl.py @@ -333,46 +333,21 @@ def test_vi_dispatches_edit(self): shell.execute("vi /tmp/script.sh") mock.assert_called_once_with("vi", ["/tmp/script.sh"]) - def test_dispatch_edit_sets_editor_env(self): - """dispatch_edit should set EDITOR to the specified editor.""" - shell = _make_shell() - captured_editor = {} - - def fake_dispatch(tokens): - import os - - captured_editor["value"] = os.environ.get("EDITOR") - - with patch.object(shell, "dispatch_contree", side_effect=fake_dispatch): - shell.dispatch_edit("nano", ["/etc/hosts"]) - - assert captured_editor["value"] == "nano" - - def test_dispatch_edit_restores_editor_env(self): - """EDITOR should be restored after _dispatch_edit.""" - import os - - shell = _make_shell() - old = os.environ.get("EDITOR") - - with patch.object(shell, "dispatch_contree"): - shell.dispatch_edit("nano", ["/etc/hosts"]) - - assert os.environ.get("EDITOR") == old - - def test_dispatch_edit_passes_file_edit_tokens(self): - """dispatch_edit should call _dispatch_contree with file edit tokens.""" + def test_dispatch_edit_passes_editor_flag(self): + """dispatch_edit should pass --editor instead of mutating env.""" shell = _make_shell() with patch.object(shell, "dispatch_contree") as mock: - shell.dispatch_edit("vim", ["/app/main.py"]) - mock.assert_called_once_with(["file", "edit", "/app/main.py"]) + shell.dispatch_edit("nano", ["/etc/hosts"]) + mock.assert_called_once_with(["file", "edit", "--editor", "nano", "/etc/hosts"]) def test_vim_relative_path_resolved(self): """'vim .bashrc' after 'cd /root' should resolve the path.""" shell = _make_shell() with _mock_session("/root"), patch.object(shell, "dispatch_contree") as mock: shell.dispatch_edit("vim", [".bashrc"]) - mock.assert_called_once_with(["file", "edit", "/root/.bashrc"]) + mock.assert_called_once_with( + ["file", "edit", "--editor", "vim", "/root/.bashrc"] + ) def test_dispatch_edit_no_path_prints_usage(self, capsys): """Editor without a path should print usage.""" @@ -943,37 +918,16 @@ def test_dispatch_contree_general_exception_logged(self, capsys): # Should not raise -class TestDispatchEditEditorRestore: - def test_editor_restored_when_previously_unset(self): - """When EDITOR was not set, it should be removed after dispatch.""" +class TestDispatchEditDoesNotMutateEnv: + def test_env_untouched(self): + """dispatch_edit forwards --editor as a flag, never mutates os.environ.""" import os shell = _make_shell() - old = os.environ.pop("EDITOR", None) - try: - with patch.object(shell, "dispatch_contree"): - shell.dispatch_edit("nano", ["/etc/hosts"]) - assert "EDITOR" not in os.environ - finally: - if old is not None: - os.environ["EDITOR"] = old - - def test_editor_restored_when_previously_set(self): - """When EDITOR was set, it should be restored after dispatch.""" - import os - - shell = _make_shell() - old = os.environ.get("EDITOR") - os.environ["EDITOR"] = "emacs" - try: - with patch.object(shell, "dispatch_contree"): - shell.dispatch_edit("nano", ["/etc/hosts"]) - assert os.environ["EDITOR"] == "emacs" - finally: - if old is not None: - os.environ["EDITOR"] = old - else: - os.environ.pop("EDITOR", None) + before = dict(os.environ) + with patch.object(shell, "dispatch_contree"): + shell.dispatch_edit("nano", ["/etc/hosts"]) + assert dict(os.environ) == before class TestNvimAlias: