diff --git a/src/leapflow/cli/commands/interactive.py b/src/leapflow/cli/commands/interactive.py index 015e2fe..9753ac3 100644 --- a/src/leapflow/cli/commands/interactive.py +++ b/src/leapflow/cli/commands/interactive.py @@ -1325,6 +1325,20 @@ async def handle_input(text: str) -> None: _apply_daemon_runtime_metadata({"host_backend": payload["result"]}) _update_status() + # A config mutation changes runtime state the status bar shows. The TUI + # is a separate process, so its cached model/context values only move + # when the daemon reports them back; without this the bar keeps showing + # the model captured at startup even though the change already applies. + if str(payload.get("view")) == "config" and payload.get("changed_keys"): + runtime_update: dict[str, Any] = {} + if payload.get("model"): + runtime_update["llm_model"] = payload["model"] + if payload.get("llm_context_length") is not None: + runtime_update["llm_context_length"] = payload["llm_context_length"] + if runtime_update: + _apply_daemon_runtime_metadata(runtime_update) + _update_status() + if str(payload.get("view")) == "dashboard": if payload.get("mode") == "open": await _open_dashboard(payload) diff --git a/src/leapflow/cli/commands/slash_handlers.py b/src/leapflow/cli/commands/slash_handlers.py index e7f529b..cca129e 100644 --- a/src/leapflow/cli/commands/slash_handlers.py +++ b/src/leapflow/cli/commands/slash_handlers.py @@ -474,7 +474,12 @@ def _config_mutation_payload(ctx: "Context", service: Any, result: Any) -> dict[ "changed_keys": list(result.changed_keys), "warnings": list(getattr(result, "warnings", ()) or ()), "reloaded": reloaded, + # Runtime values the status bar renders. A daemon-mode TUI is a separate + # process and cannot see the reload, so it needs them echoed back here; + # context length travels with the model because switching models usually + # changes it too. "model": ctx.settings.llm_model, + "llm_context_length": int(getattr(ctx.settings, "llm_context_length", 0) or 0), } diff --git a/src/leapflow/cli/context.py b/src/leapflow/cli/context.py index 320ba57..13f4196 100644 --- a/src/leapflow/cli/context.py +++ b/src/leapflow/cli/context.py @@ -1248,6 +1248,13 @@ async def initialize_critical(self) -> None: from leapflow.tools.registry_bootstrap import set_memory_manager set_memory_manager(self.memory) + # ── Config tools: bind this Context so a config write reloads the live + # session, the same way `/config set` does. Without it the write lands on + # disk while the in-process settings keep the old value, and an immediate + # read-back looks like the write failed. + from leapflow.tools.config_tools import set_config_context + set_config_context(self) + # ── Gateway server (late-bound tool wiring) ── from leapflow.gateway.server import GatewayServer from leapflow.gateway.router import GatewayRouter @@ -1341,6 +1348,11 @@ async def _on_gateway_event_with_bridge(event: object) -> None: self.gateway_server.discover_manifests() set_gateway_server(self.gateway_server) set_gateway_approval_gate(self._approval_orchestrator) + # Config writes are gated too: several writable keys weaken safety + # machinery (guardrail.enabled, confirm.default_level, codegen.sandbox), + # so config_set must not be able to disable its own supervision. + from leapflow.tools.config_tools import set_config_approval_gate + set_config_approval_gate(self._approval_orchestrator) self._register_gateway_normalizers(settings) diff --git a/src/leapflow/daemon/_service_helpers.py b/src/leapflow/daemon/_service_helpers.py index 07c3a48..73495a0 100644 --- a/src/leapflow/daemon/_service_helpers.py +++ b/src/leapflow/daemon/_service_helpers.py @@ -38,12 +38,22 @@ def memory_entry_to_dict(entry: MemoryEntry) -> dict[str, Any]: # ── Engine / context metadata ──────────────────────────────────────── def engine_context_metadata(engine: Any | None, settings: Any) -> dict[str, Any]: - """Return safe context-budget metadata for daemon status and stream events.""" + """Return safe context-budget metadata for daemon status and stream events. + + ``llm_model`` rides along because the TUI is a separate process: its status + bar seeds the model name at startup and can only learn about a change from + metadata the daemon sends back. Deriving it here means every status/stream + path reports the model actually in use, including after a mid-turn + ``config_set``, instead of relying on a one-off change notification. + """ context_length = max(0, int(getattr(settings, "llm_context_length", 0) or 0)) metadata: dict[str, Any] = { "llm_context_length": context_length, "context_used": 0, } + model = str(getattr(settings, "llm_model", "") or "") + if model: + metadata["llm_model"] = model if engine is None: return metadata metadata["context_used"] = max(0, int(getattr(engine, "context_token_count", 0) or 0)) diff --git a/src/leapflow/daemon/approval_coordinator.py b/src/leapflow/daemon/approval_coordinator.py index 0880af6..f12cb29 100644 --- a/src/leapflow/daemon/approval_coordinator.py +++ b/src/leapflow/daemon/approval_coordinator.py @@ -29,6 +29,7 @@ def install_gate(self, ctx: Any, service: Any) -> None: from leapflow.security.approval import SessionAwareGate from leapflow.security.actions import ActionDescriptor from leapflow.security.orchestrator import ApprovalOrchestrator + from leapflow.tools.config_tools import set_config_approval_gate from leapflow.tools.gateway_tool import set_gateway_approval_gate from leapflow.tools.registry_bootstrap import set_file_read_gate, set_file_write_gate from leapflow.tools.shell_tools import set_approval_gate @@ -44,6 +45,9 @@ def install_gate(self, ctx: Any, service: Any) -> None: ctx._approval_orchestrator = orchestrator set_approval_gate(orchestrator) set_gateway_approval_gate(orchestrator) + # Config writes go through the same daemon-side approval path, so a + # daemon session cannot change settings unattended either. + set_config_approval_gate(orchestrator) class _FileReadGate: def __init__(self) -> None: diff --git a/src/leapflow/layout.py b/src/leapflow/layout.py index 1a29006..cf3e5cd 100644 --- a/src/leapflow/layout.py +++ b/src/leapflow/layout.py @@ -511,11 +511,22 @@ def workspace_config_path(self, workspace_root: Path) -> Path: def workspace_manifest_path(self, workspace_root: Path) -> Path: return workspace_root / ".leapflow" / "workspace.yaml" - def write_workspace_manifest(self, workspace_root: Path) -> Path: - """Write the workspace-local manifest and return its path.""" + def write_workspace_manifest(self, workspace_root: Path) -> Path | None: + """Write the workspace-local manifest; return its path, or ``None`` if skipped. + + Skipped when the workspace's ``.leapflow`` directory *is* the LeapFlow + home (i.e. the workspace root is the home's parent, typically ``$HOME``). + Writing there would drop a workspace marker into the global home + alongside ``config/``, ``profiles/`` and ``secrets/`` — the two + directories collapse onto one path and the layout stops being readable. + The profile-side manifest (addressed by ``workspace_id``) is written + unconditionally by the caller, so nothing is lost by skipping this copy. + """ workspace_root = workspace_root.expanduser().resolve() - workspace_id = workspace_id_for_path(workspace_root) path = self.workspace_manifest_path(workspace_root) + if path.parent.resolve() == self.root.resolve(): + return None + workspace_id = workspace_id_for_path(workspace_root) now = datetime.now(timezone.utc).isoformat() existing: dict[str, Any] = {} if path.exists(): diff --git a/src/leapflow/tools/config_tools.py b/src/leapflow/tools/config_tools.py new file mode 100644 index 0000000..4cb93dd --- /dev/null +++ b/src/leapflow/tools/config_tools.py @@ -0,0 +1,336 @@ +"""Config tools: let the model read and change settings without touching paths. + +Without these, a request like "switch the model to X" has no legal path: the +model has only ``file_read`` / ``shell_run``, so it guesses at +``~/.leapflow/...`` and the workspace sandbox correctly refuses. Telling it +"do not probe that path" in a description only moves the guess elsewhere — the +goal is unchanged while no capability exists to serve it. + +These tools close that gap by delegating to ``ConfigService``, the same control +plane behind ``leap config`` and ``/config``. They take a key, never a path, so +the sandbox is never involved and the layout stays an implementation detail. +Routing writes through the service (rather than letting the model edit YAML) +also keeps type coercion, scope validation, vault-backed secrets, and +hot-reload semantics intact. +""" + +from __future__ import annotations + +import difflib +import logging +from typing import Any, Dict + +logger = logging.getLogger(__name__) + +# A listing of every writable field is long; keep the default bounded and let the +# model narrow by category (categories come back in the payload either way). +_DEFAULT_LIST_LIMIT = 60 + +# Set by the CLI/daemon so a write can hot-reload the live session, the same way +# ``/config set`` does. Without it a write lands on disk while the in-process +# Settings singleton keeps the old value, and the model's read-back shows the +# stale value and looks like a failed write. +_context_ref: Any = None + +# Approval gate for writes. ``requires_approval`` in the tool's x_leapflow block +# only informs capability disclosure — it does not gate execution — so a write +# must consult this explicitly, the same way shell/file_write do. Several writable +# keys weaken safety machinery (``guardrail.enabled``, ``confirm.default_level``, +# ``codegen.sandbox``), so an unguarded config_set would let the model disable its +# own supervision. +_approval_gate: Any = None + + +def set_config_context(ctx: Any) -> None: + """Bind the runtime Context so config writes can reload the live session.""" + global _context_ref + _context_ref = ctx + + +def set_config_approval_gate(gate: Any) -> None: + """Install the approval gate consulted before a config write.""" + global _approval_gate + _approval_gate = gate + + +def get_config_approval_gate() -> Any: + """Return the installed config approval gate (or ``None``).""" + return _approval_gate + + +def _active_settings() -> Any: + """Return the live session's settings, falling back to the global singleton.""" + settings = getattr(_context_ref, "settings", None) if _context_ref is not None else None + if settings is not None: + return settings + from leapflow.config import get_settings + + return get_settings() + + +def _reload_after_write() -> bool: + """Apply a persisted change to the running session; return whether it took.""" + reload_fn = getattr(_context_ref, "reload_runtime_config_if_changed", None) + if reload_fn is None: + return False + try: + return bool(reload_fn(force=True)) + except Exception: # noqa: BLE001 - a failed reload must not undo a valid write + logger.debug("config write: session reload failed", exc_info=True) + return False + + +def _service() -> Any: + """Build a ConfigService over the active settings.""" + from leapflow.config_service import ConfigService + + return ConfigService(_active_settings()) + + +def _field_payload(view: Any, *, include_description: bool = True) -> Dict[str, Any]: + """Render a ConfigFieldView for the model. + + ``hot_reload`` is always included: a ``restart-required`` field that appears + to change but does not take effect is the most confusing outcome of a config + edit, so the model must be able to tell the user. + """ + payload: Dict[str, Any] = { + "key": view.key, + "value": view.value, + "value_type": getattr(view.value_type, "__name__", str(view.value_type)), + "category": view.category, + "scopes": list(view.scopes), + "hot_reload": view.hot_reload, + "secret": bool(view.secret), + } + if include_description and view.description: + payload["description"] = view.description + if view.value_hint: + payload["value_hint"] = view.value_hint + if view.examples: + payload["examples"] = list(view.examples) + return payload + + +async def config_list_handler(args: Dict[str, Any]) -> Dict[str, Any]: + """List writable config fields, optionally narrowed to one category.""" + category = str(args.get("category") or "").strip() or None + try: + limit = max(1, int(args.get("limit") or _DEFAULT_LIST_LIMIT)) + except (TypeError, ValueError): + limit = _DEFAULT_LIST_LIMIT + + try: + service = _service() + views = service.list_fields(category) + except Exception as exc: # noqa: BLE001 - surfaced as a tool failure, not a crash + logger.debug("config_list failed", exc_info=True) + return {"ok": False, "error": f"Could not read the config catalog: {exc}", "retryable": False} + + categories = sorted({view.category for view in service.list_fields(None)}) + if category and not views: + return { + "ok": False, + "error": f"No config fields in category {category!r}.", + "available_categories": categories, + "retryable": True, + } + + truncated = len(views) > limit + return { + "ok": True, + "total": len(views), + "returned": min(len(views), limit), + "truncated": truncated, + "categories": categories, + "fields": [ + _field_payload(view, include_description=bool(category)) + for view in views[:limit] + ], + } + + +async def config_get_handler(args: Dict[str, Any]) -> Dict[str, Any]: + """Return one config field with its value and semantics.""" + key = str(args.get("key") or "").strip() + if not key: + return {"ok": False, "error": "config_get requires 'key'.", "retryable": True} + + try: + service = _service() + view = service.describe(key) + except ValueError as exc: + # Unknown key is recoverable in the same turn: hand back near matches so + # the model can correct itself instead of falling back to file probing. + return { + "ok": False, + "error": str(exc), + "retryable": True, + "did_you_mean": _suggest(key), + } + except Exception as exc: # noqa: BLE001 + logger.debug("config_get failed for %s", key, exc_info=True) + return {"ok": False, "error": f"Could not read config key {key!r}: {exc}", "retryable": False} + + payload = _field_payload(view) + payload.update({"ok": True}) + return payload + + +async def _approve_write(key: str, *, scope: str, secret: bool, hot_reload: str) -> str: + """Return a denial reason, or ``""`` when the write may proceed. + + Goes through the orchestrator's native ``evaluate(ActionDescriptor)`` rather + than the shell-oriented ``check()``, so a config change is classified as + ``runtime.configure`` and picks up risk assessment, policy, existing grants, + and the audit trail for free. + + Fails closed when no gate is installed: an unguarded path here would let the + model turn off its own guardrails. The value is never included, so a + credential cannot reach an approval prompt or the audit log. + """ + gate = _approval_gate + if gate is None: + return ( + "Config changes require an approval gate, which is not available in this " + "session. Ask the user to run `leap config set` / `/config set` instead." + ) + try: + from leapflow.security.actions import ActionDescriptor, ActionEffect, ActionKind + + action = ActionDescriptor( + kind=ActionKind.RUNTIME_CONFIGURE.value, + summary=f"Change LeapFlow setting {key} (scope={scope})", + detail=f"config_set {key} scope={scope}", + effect=ActionEffect.CONFIGURE.value, + resource=key, + metadata={ + "tool": "config_set", + "config_key": key, + "scope": scope, + "secret": secret, + "hot_reload": hot_reload, + }, + ) + result = await gate.evaluate(action) + except Exception: # noqa: BLE001 - a broken gate must not become an open door + logger.warning("config_set: approval evaluation failed; denying", exc_info=True) + return "Config change denied: the approval gate could not be consulted." + + if getattr(result, "approved", False): + return "" + return str( + getattr(result, "denial_message", "") + or getattr(result, "reason", "") + or f"Config change denied by approval gate: {key}" + ) + + +async def config_set_handler(args: Dict[str, Any]) -> Dict[str, Any]: + """Write one config field through ConfigService.""" + key = str(args.get("key") or "").strip() + if not key: + return {"ok": False, "error": "config_set requires 'key'.", "retryable": True} + if "value" not in args: + return {"ok": False, "error": "config_set requires 'value'.", "retryable": True} + scope = str(args.get("scope") or "profile").strip() or "profile" + + try: + service = _service() + before = service.describe(key) + except ValueError as exc: + return { + "ok": False, + "error": str(exc), + "retryable": True, + "did_you_mean": _suggest(key), + } + except Exception as exc: # noqa: BLE001 + logger.debug("config_set failed to describe %s", key, exc_info=True) + return {"ok": False, "error": f"Could not read config key {key!r}: {exc}", "retryable": False} + + denial = await _approve_write( + key, scope=scope, secret=bool(before.secret), hot_reload=before.hot_reload, + ) + if denial: + return {"ok": False, "error": denial, "retryable": False, "requires_approval": True} + + try: + result = service.set(key, args["value"], scope=scope) # type: ignore[arg-type] + except ValueError as exc: + return {"ok": False, "error": str(exc), "retryable": True} + except Exception as exc: # noqa: BLE001 + logger.debug("config_set failed for %s", key, exc_info=True) + return {"ok": False, "error": f"Could not set config key {key!r}: {exc}", "retryable": False} + + payload: Dict[str, Any] = { + "ok": bool(result.ok), + "key": key, + "scope": scope, + "message": result.message, + "changed_keys": list(result.changed_keys), + "hot_reload": before.hot_reload, + } + if result.warnings: + payload["warnings"] = list(result.warnings) + # Never echo a credential back into the transcript. + if not before.secret: + payload["value"] = args["value"] + if before.hot_reload == "restart-required": + payload["restart_required"] = True + payload["next_step"] = "Run `leap daemon restart` for this change to take effect." + elif result.ok: + # Reload so an immediate config_get reflects the new value; otherwise the + # model sees the stale singleton and concludes the write failed. + payload["session_reloaded"] = _reload_after_write() + return payload + + +def _suggest(key: str, *, limit: int = 5) -> list[str]: + """Return catalog keys resembling ``key``. + + Substring matching alone is not enough: the realistic mistakes are typos + (``llm.modle``) and dropped separators (``daemon.loglevel``), which share no + substring with the real key. Fuzzy matching on both the full key and its last + segment covers those, so the model can correct itself in the same turn + instead of falling back to probing files. + """ + needle = str(key or "").strip().lower() + if not needle: + return [] + try: + candidates = list(_service().writable_keys()) + except Exception: # noqa: BLE001 - suggestions are best-effort + return [] + + ranked: list[str] = [] + # Substring hits first: an exact fragment is a stronger signal than similarity. + ranked.extend(c for c in candidates if needle in c.lower()) + + compact = needle.replace("_", "").replace("-", "").replace(".", "") + ranked.extend( + c for c in candidates + if c.lower().replace("_", "").replace(".", "") == compact + ) + ranked.extend(difflib.get_close_matches(needle, candidates, n=limit, cutoff=0.6)) + + tail = needle.rsplit(".", 1)[-1] + if tail and tail != needle: + tails = {c: c.rsplit(".", 1)[-1] for c in candidates} + ranked.extend(c for c, t in tails.items() if t == tail) + close_tails = set(difflib.get_close_matches(tail, list(tails.values()), n=limit, cutoff=0.7)) + ranked.extend(c for c, t in tails.items() if t in close_tails) + + seen: set[str] = set() + ordered = [c for c in ranked if not (c in seen or seen.add(c))] + return ordered[:limit] + + +__all__ = [ + "config_get_handler", + "config_list_handler", + "config_set_handler", + "get_config_approval_gate", + "set_config_approval_gate", + "set_config_context", +] diff --git a/src/leapflow/tools/execution_context.py b/src/leapflow/tools/execution_context.py index 8e41f06..57b8a88 100644 --- a/src/leapflow/tools/execution_context.py +++ b/src/leapflow/tools/execution_context.py @@ -90,17 +90,56 @@ def is_within_allowed_roots(path: Path, ctx: ToolExecutionContext | None = None) return False +def _leapflow_managed_hint(path: Path) -> str: + """Return a redirect hint when ``path`` is LeapFlow's own managed state. + + A refusal that only says "outside the workspace" leaves the model to guess + another path, which is how a config change turns into a sequence of blocked + probes. Classification comes from the layout descriptor rather than string + matching, so it follows the path tree instead of duplicating it. + """ + try: + from leapflow.config import get_settings + + layout = getattr(get_settings(), "layout", None) + if layout is None: + return "" + descriptor = layout.describe_path(path) + except Exception: # noqa: BLE001 - a hint must never break the refusal itself + return "" + + category = str(getattr(descriptor, "category", "") or "") + if category in {"config", "mcp_config", "workspace_manifest"}: + return ( + " This is LeapFlow's own configuration: use the config_list / config_get / " + "config_set tools instead of reading or editing these files." + ) + if category == "secret_vault": + return ( + " This is LeapFlow's credential vault and is never readable as a file: " + "set credentials with config_set (e.g. key='llm.api_key')." + ) + return "" + + def workspace_scope_error(path: Path, *, operation: str) -> dict[str, Any] | None: - """Return a structured error when ``path`` escapes the active workspace.""" + """Return a structured error when ``path`` escapes the active workspace. + + The workspace boundary is a hard gate evaluated at the tool entry point: it + is deliberately not routed through ApprovalGate, so the message must not + imply that approving something will unblock it. + """ ctx = current_tool_context() if ctx is None or is_within_allowed_roots(path, ctx): return None + hint = _leapflow_managed_hint(path) return { "ok": False, "error": ( f"{operation} path is outside the active workspace. " f"Resolved path: {path}; workspace root: {ctx.workspace_root}. " - "Open a TUI in that workspace or ask explicitly for an external path with approval." + "This boundary cannot be lifted by approval; work inside the workspace, " + "or ask the user to open a session in that directory." + hint ), "error_type": "outside_workspace", "retryable": False, diff --git a/src/leapflow/tools/registry_bootstrap.py b/src/leapflow/tools/registry_bootstrap.py index 79e020c..cf16558 100644 --- a/src/leapflow/tools/registry_bootstrap.py +++ b/src/leapflow/tools/registry_bootstrap.py @@ -76,10 +76,8 @@ "Read text file content with adaptive context governance. For large or unfamiliar files, " "prefer mode='outline' or mode='symbols' first, then use mode='raw' " "with start_line/max_lines for the specific range you actually need. " - "Do not probe `/.leapflow/config.json`; LeapFlow uses structured " - "config under `~/.leapflow/config/user.yaml`, " - "`~/.leapflow/profiles//config/*.yaml`, and optional " - "`/.leapflow/config.yaml`." + "For LeapFlow's own settings, use config_list / config_get / config_set — " + "its config files are outside the workspace and not readable here." ), "parameters": { "type": "object", @@ -693,6 +691,80 @@ "x_leapflow": {"category": "terminal", "risk_level": "read_only", "schema_cost": "low", "requires_approval": False}, }, }, + # ── LeapFlow settings (never read config files; these take keys, not paths) ── + { + "type": "function", + "function": { + "name": "config_list", + "description": ( + "List LeapFlow's own writable settings (model, provider, daemon, memory, " + "perception, gateway, …) with current values. Use this to discover the exact " + "key before changing anything. Optionally narrow by `category`. This is the " + "only correct way to inspect LeapFlow configuration — never read config files " + "from disk." + ), + "parameters": { + "type": "object", + "properties": { + "category": {"type": "string", "description": "Optional category filter, e.g. 'LLM Provider' or 'Runtime'"}, + "limit": {"type": "integer", "description": "Max fields to return (default 60)"}, + }, + }, + "x_leapflow": {"category": "config", "risk_level": "read_only", "schema_cost": "low", "requires_approval": False}, + }, + }, + { + "type": "function", + "function": { + "name": "config_get", + "description": ( + "Read one LeapFlow setting by key (e.g. 'llm.model', 'daemon.log_level'), " + "returning its current value, type, scopes, and whether a change needs a " + "daemon restart. Never read LeapFlow config files from disk — use this." + ), + "parameters": { + "type": "object", + "properties": { + "key": {"type": "string", "description": "Dot-separated config key, e.g. 'llm.model'"}, + }, + "required": ["key"], + }, + "x_leapflow": {"category": "config", "risk_level": "read_only", "schema_cost": "low", "requires_approval": False}, + }, + }, + { + "type": "function", + "function": { + "name": "config_set", + "description": ( + "Change one LeapFlow setting by key, e.g. switch the model with " + "key='llm.model'. Values are validated and coerced; credentials are stored in " + "the vault automatically. Call config_list or config_get first if unsure of " + "the key. The result states whether a `leap daemon restart` is required. " + "Never edit LeapFlow config files directly." + ), + "parameters": { + "type": "object", + "properties": { + "key": {"type": "string", "description": "Dot-separated config key, e.g. 'llm.model'"}, + "value": {"description": "New value; coerced to the field's declared type"}, + "scope": {"type": "string", "enum": ["profile", "workspace"], "description": "Where to persist (default: profile)"}, + }, + "required": ["key", "value"], + }, + "x_leapflow": { + "category": "config", + "risk_level": "medium", + "schema_cost": "low", + "requires_approval": True, + "mutates_state": True, + # Re-setting the same value converges, so a replay is safe: this keeps + # the change out of the uncertain-effect path that would otherwise stall + # a legitimate retry. + "idempotency_scope": "turn", + }, + }, + }, ] + HUB_TOOL_DEFINITIONS + GATEWAY_TOOL_DEFINITIONS @@ -1062,6 +1134,27 @@ async def _memory_add_handler(params: Dict[str, Any]) -> Dict[str, Any]: TOOL_HANDLERS["gp_memory_add"] = _memory_add_handler +# ──────────────────────────────────────────────────────────────── +# LeapFlow settings: delegate to ConfigService so the model changes settings by +# key instead of guessing at config file paths (which the workspace sandbox +# rightly refuses). Registered here alongside the other late-bound handlers. +# ──────────────────────────────────────────────────────────────── + +from leapflow.tools.config_tools import ( # noqa: E402 - late import keeps module import cheap + config_get_handler as _config_get_handler, + config_list_handler as _config_list_handler, + config_set_handler as _config_set_handler, +) + +for _cfg_name, _cfg_handler in ( + ("config_list", _config_list_handler), + ("config_get", _config_get_handler), + ("config_set", _config_set_handler), +): + TOOL_HANDLERS[_cfg_name] = _cfg_handler + TOOL_HANDLERS[f"gp_{_cfg_name}"] = _cfg_handler + + # ──────────────────────────────────────────────────── # Research-ledger tool late-binding: delegates to the engine's per-task # ResearchLedger when installed; fails gracefully when not. diff --git a/src/leapflow/version.py b/src/leapflow/version.py index 4004170..929f491 100644 --- a/src/leapflow/version.py +++ b/src/leapflow/version.py @@ -1,3 +1,3 @@ """Version information for leapflow.""" -__version__ = "0.0.6+main" +__version__ = "0.0.7+main" diff --git a/tests/test_cli_entrypoint.py b/tests/test_cli_entrypoint.py index ea60a9e..58d4674 100644 --- a/tests/test_cli_entrypoint.py +++ b/tests/test_cli_entrypoint.py @@ -44,6 +44,11 @@ async def test_config_slash_updates_model_and_hot_reloads(tmp_path) -> None: assert payload["reloaded"] is True assert ctx.settings.llm_model == "qwen3.7-plus" assert "model: qwen3.7-plus" in ctx.settings.profile_layout.llm_config_path.read_text(encoding="utf-8") + # The status bar lives in a separate TUI process and can only learn the new + # model from this payload, so a mutation must echo the runtime values back. + assert payload["model"] == "qwen3.7-plus" + assert payload["llm_context_length"] == int(ctx.settings.llm_context_length) + assert payload["changed_keys"], "the TUI keys its status refresh off changed_keys" payload = await command_execute(ctx, "config", "set memory.working_max_tokens 12000") assert payload["ok"] is True diff --git a/tests/test_config_capability_tools.py b/tests/test_config_capability_tools.py new file mode 100644 index 0000000..f648982 --- /dev/null +++ b/tests/test_config_capability_tools.py @@ -0,0 +1,497 @@ +"""Guards for the config capability and the path boundaries around it. + +The scenario these lock down: a user asks to change ``llm.model`` from inside an +arbitrary workspace. Before the ``config_*`` tools existed the model had only +``file_read`` / ``shell_run``, so it guessed at ``~/.leapflow/...`` and the +workspace sandbox refused every attempt — with a message that wrongly implied +approval could lift the boundary. + +Covered: +- config tools work by key and never depend on a workspace/tool context +- reads reach the model by default; writes stay behind disclosure and approval +- a write is visible to an immediate read-back (no stale singleton) +- credentials never echo into the transcript +- unknown keys are self-correctable in the same turn +- the sandbox refusal is honest about approval and points at the config tools +- the workspace manifest never lands inside the LeapFlow home +""" + +from __future__ import annotations + +import asyncio +from pathlib import Path +from typing import Any + +import pytest + +from leapflow.layout import build_layout +from leapflow.tools import config_tools + + +@pytest.fixture() +def cfg_home(monkeypatch, tmp_path): + """Point LeapFlow at a temp home, reset the settings singleton, allow writes. + + ``config_set`` fails closed without an approval gate, so tests that exercise a + write install a permissive one here; the gate's own behavior is covered + separately below. + """ + import leapflow.config as config_module + + monkeypatch.setenv("LEAPFLOW_DATA_DIR", str(tmp_path / "home")) + monkeypatch.setattr(config_module, "_settings_instance", None, raising=False) + config_tools.set_config_context(None) + config_tools.set_config_approval_gate(_AllowGate()) + yield tmp_path / "home" + monkeypatch.setattr(config_module, "_settings_instance", None, raising=False) + config_tools.set_config_context(None) + config_tools.set_config_approval_gate(None) + + +class _Result: + """Mirrors the fields of security.orchestrator.ApprovalResult that we read.""" + + def __init__(self, approved: bool, denial_message: str = "") -> None: + self.approved = approved + self.denial_message = denial_message + self.reason = denial_message + + +class _AllowGate: + """Approves every config write and records the action it was asked about. + + Implements ``evaluate(ActionDescriptor)`` — the orchestrator's real interface. + An earlier version of this fake accepted the shell-style ``check(...)`` + signature, which let the tests pass while the production call raised + TypeError against the actual orchestrator. + """ + + def __init__(self) -> None: + self.actions: list[Any] = [] + + async def evaluate(self, action): + self.actions.append(action) + return _Result(True) + + +class _DenyGate: + def __init__(self, message: str = "denied for test") -> None: + self.message = message + + async def evaluate(self, action): + return _Result(False, self.message) + + +class _ReloadingContext: + """Minimal stand-in for the runtime Context's hot-reload contract.""" + + def __init__(self) -> None: + from leapflow.config import load_config + + self.settings = load_config() + self.reloads = 0 + + def reload_runtime_config_if_changed(self, *, force: bool = False) -> bool: + from leapflow.config import load_config + + self.settings = load_config() + self.reloads += 1 + return True + + +# ── Reading and writing by key, never by path ──────────────────────────── + + +def test_config_list_reports_categories_and_bounded_fields(cfg_home) -> None: + result = asyncio.run(config_tools.config_list_handler({"limit": 5})) + + assert result["ok"] is True + assert result["returned"] == 5 and result["total"] > 5 + assert result["truncated"] is True + assert "LLM Provider" in result["categories"] + + +def test_config_list_rejects_unknown_category_with_the_valid_set(cfg_home) -> None: + """A wrong filter must be recoverable, not a dead end.""" + result = asyncio.run(config_tools.config_list_handler({"category": "Nope"})) + + assert result["ok"] is False and result["retryable"] is True + assert result["available_categories"] + + +def test_config_get_exposes_hot_reload_semantics(cfg_home) -> None: + """Without this the user changes a restart-required field and sees no effect.""" + result = asyncio.run(config_tools.config_get_handler({"key": "daemon.log_level"})) + + assert result["ok"] is True + assert result["hot_reload"] == "restart-required" + assert result["description"] + + +def test_config_set_changes_the_value_and_read_back_agrees(cfg_home) -> None: + """The original scenario: switch the model and confirm it took.""" + config_tools.set_config_context(_ReloadingContext()) + + written = asyncio.run( + config_tools.config_set_handler({"key": "llm.model", "value": "qwen3.8-max"}) + ) + read_back = asyncio.run(config_tools.config_get_handler({"key": "llm.model"})) + + assert written["ok"] is True + assert written["changed_keys"] == ["llm.model"] + assert written["session_reloaded"] is True + # The whole point: a stale singleton here reads as a failed write. + assert read_back["value"] == "qwen3.8-max" + + +def test_config_set_reports_restart_requirement(cfg_home) -> None: + result = asyncio.run( + config_tools.config_set_handler({"key": "daemon.log_level", "value": "DEBUG"}) + ) + + assert result["ok"] is True + assert result["restart_required"] is True + assert "restart" in result["next_step"].lower() + + +def test_config_set_never_echoes_a_secret(cfg_home) -> None: + """A credential in the tool result would land in the transcript.""" + result = asyncio.run( + config_tools.config_set_handler({"key": "llm.api_key", "value": "sk-must-not-echo"}) + ) + + assert result["ok"] is True + assert "value" not in result + assert "sk-must-not-echo" not in str(result) + + +def test_config_set_requires_key_and_value(cfg_home) -> None: + missing_value = asyncio.run(config_tools.config_set_handler({"key": "llm.model"})) + missing_key = asyncio.run(config_tools.config_set_handler({"value": "x"})) + + assert missing_value["ok"] is False and missing_value["retryable"] is True + assert missing_key["ok"] is False and missing_key["retryable"] is True + + +@pytest.mark.parametrize( + ("typo", "expected"), + [ + ("llm.modle", "llm.model"), # transposed letters + ("daemon.loglevel", "daemon.log_level"), # dropped separator + ("llm.api-key", "llm.api_key"), # hyphen instead of underscore + ("model", "llm.model"), # bare last segment + ], +) +def test_unknown_key_suggests_the_real_one(cfg_home, typo: str, expected: str) -> None: + """Self-correction in the same turn is what prevents a fallback to probing.""" + result = asyncio.run(config_tools.config_get_handler({"key": typo})) + + assert result["ok"] is False and result["retryable"] is True + assert expected in result["did_you_mean"] + + +def test_config_tools_ignore_the_workspace_boundary(cfg_home) -> None: + """They take keys, so an unrelated workspace context must not affect them. + + This is why the sandbox never has to be relaxed for configuration. + """ + from leapflow.tools.execution_context import ( + ToolExecutionContext, + reset_tool_context, + set_tool_context, + ) + + ctx = ToolExecutionContext.from_strings(workspace_root=str(cfg_home.parent / "elsewhere")) + token = set_tool_context(ctx) + try: + result = asyncio.run(config_tools.config_get_handler({"key": "llm.model"})) + finally: + reset_tool_context(token) + + assert result["ok"] is True + + +# ── Sandbox refusal: honest, and pointing somewhere useful ─────────────── + + +def test_sandbox_refusal_does_not_promise_approval(cfg_home) -> None: + """The boundary is a hard gate; claiming approval helps sends users in circles.""" + from leapflow.tools.execution_context import ( + ToolExecutionContext, + reset_tool_context, + set_tool_context, + workspace_scope_error, + ) + + token = set_tool_context( + ToolExecutionContext.from_strings(workspace_root=str(cfg_home.parent / "ws")) + ) + try: + error = workspace_scope_error(cfg_home / "config" / "user.yaml", operation="file_read") + finally: + reset_tool_context(token) + + assert error is not None + assert "cannot be lifted by approval" in error["error"] + assert "with approval" not in error["error"].replace("cannot be lifted by approval", "") + + +def test_sandbox_refusal_redirects_config_paths_to_the_tools(cfg_home) -> None: + """A refusal that names the right capability ends the guessing loop.""" + from leapflow.tools.execution_context import ( + ToolExecutionContext, + reset_tool_context, + set_tool_context, + workspace_scope_error, + ) + + build_layout(cfg_home).ensure(profile_id="default") + token = set_tool_context( + ToolExecutionContext.from_strings(workspace_root=str(cfg_home.parent / "ws")) + ) + try: + config_error = workspace_scope_error( + cfg_home / "config" / "user.yaml", operation="file_read" + ) + vault_error = workspace_scope_error( + cfg_home / "secrets" / "vault.key", operation="file_read" + ) + plain_error = workspace_scope_error( + cfg_home.parent / "unrelated" / "notes.txt", operation="file_read" + ) + finally: + reset_tool_context(token) + + assert "config_set" in config_error["error"] + assert "config_set" in vault_error["error"] + # An ordinary outside-workspace path gets no config advice. + assert "config_set" not in plain_error["error"] + + +# ── Workspace manifest must not land in the LeapFlow home ──────────────── + + +def test_manifest_is_skipped_when_workspace_is_the_leapflow_home(tmp_path) -> None: + """Writing here would drop a workspace marker beside config/ and secrets/.""" + home = tmp_path / ".leapflow" + layout = build_layout(home) + + written = layout.write_workspace_manifest(tmp_path) + + assert written is None + assert not (home / "workspace.yaml").exists() + + +def test_manifest_is_written_for_a_normal_workspace(tmp_path) -> None: + layout = build_layout(tmp_path / "leap-home") + workspace = tmp_path / "project" + workspace.mkdir() + + written = layout.write_workspace_manifest(workspace) + + assert written == workspace / ".leapflow" / "workspace.yaml" + assert Path(written).exists() + + +# ── Writes are gated: the model must not be able to unsupervise itself ───── + + +def _guardrail_is_on() -> bool: + """Whether the guardrail is still enabled. + + Compares loosely on purpose: the effective value surfaces as a string + (``'true'``) rather than a bool, and what matters here is only that the write + did not switch it off. + """ + value = asyncio.run(config_tools.config_get_handler({"key": "guardrail.enabled"}))["value"] + return str(value).strip().lower() in {"true", "1", "yes"} + + +def test_config_write_is_denied_without_an_approval_gate(cfg_home) -> None: + """Fail closed. ``requires_approval`` in the tool schema only drives capability + disclosure — it does not gate execution — so an unwired gate must block, not + silently allow. + """ + config_tools.set_config_approval_gate(None) + + result = asyncio.run( + config_tools.config_set_handler({"key": "guardrail.enabled", "value": False}) + ) + + assert result["ok"] is False + assert result["requires_approval"] is True + assert _guardrail_is_on(), "a denied write must not reach disk" + + +def test_config_write_honors_gate_denial(cfg_home) -> None: + config_tools.set_config_approval_gate(_DenyGate()) + + result = asyncio.run( + config_tools.config_set_handler({"key": "llm.model", "value": "sneaky"}) + ) + + assert result["ok"] is False + assert "denied for test" in result["error"] + assert asyncio.run(config_tools.config_get_handler({"key": "llm.model"}))["value"] != "sneaky" + + +def test_gate_sees_the_key_but_never_the_value(cfg_home) -> None: + """An approval prompt and its audit trail must not carry a credential.""" + gate = _AllowGate() + config_tools.set_config_approval_gate(gate) + + asyncio.run( + config_tools.config_set_handler({"key": "llm.api_key", "value": "sk-secret-value"}) + ) + + assert gate.actions + action = gate.actions[-1] + assert action.resource == "llm.api_key" + assert action.kind == "runtime.configure" + assert action.metadata["secret"] is True + assert "sk-secret-value" not in f"{action.summary}{action.detail}{action.metadata}" + + +def test_gate_is_called_through_its_real_interface(cfg_home) -> None: + """Regression: the tool must use evaluate(ActionDescriptor), not check(). + + ApprovalOrchestrator.check() takes a single command string, so calling it with + the file-write signature raised TypeError at runtime while a fake accepting + that signature kept the tests green. + """ + from leapflow.security.orchestrator import ApprovalOrchestrator + + assert hasattr(ApprovalOrchestrator, "evaluate") + gate = _AllowGate() + config_tools.set_config_approval_gate(gate) + + result = asyncio.run( + config_tools.config_set_handler({"key": "llm.model", "value": "qwen3.8-max"}) + ) + + assert result["ok"] is True + assert gate.actions, "the gate must be consulted via evaluate()" + + +def test_a_gate_with_only_the_shell_signature_is_denied(cfg_home) -> None: + """A gate lacking evaluate() must fail closed, not silently allow.""" + + class _ShellOnlyGate: + async def check(self, command): + return True + + config_tools.set_config_approval_gate(_ShellOnlyGate()) + + result = asyncio.run( + config_tools.config_set_handler({"key": "guardrail.enabled", "value": False}) + ) + + assert result["ok"] is False + assert _guardrail_is_on() + + +def test_write_works_against_the_real_orchestrator(cfg_home) -> None: + """End-to-end through the production ApprovalOrchestrator, not a fake. + + This is the check the fakes could not make: the tool previously called the + shell-oriented ``check(path, content, mode, meta)``, which the orchestrator + does not accept, so every config_set failed with a TypeError at runtime while + a signature-matching fake kept the suite green. It also confirms a config + change is classified ``runtime.configure`` and rated HIGH, so it reaches a + human rather than being auto-allowed. + """ + from leapflow.security.approval import ApprovalDecision + from leapflow.security.orchestrator import ApprovalOrchestrator + + class _AutoAllow: + def __init__(self) -> None: + self.requests: list[Any] = [] + + async def request_approval(self, request): + self.requests.append(request) + return ApprovalDecision.ALLOW_ONCE + + prompt = _AutoAllow() + config_tools.set_config_approval_gate(ApprovalOrchestrator(prompt)) + # Bind a reloading context so the read-back sees the write; without it the + # in-process settings keep the old value (covered separately above). + config_tools.set_config_context(_ReloadingContext()) + + result = asyncio.run( + config_tools.config_set_handler({"key": "llm.model", "value": "qwen3.8-max"}) + ) + + assert result["ok"] is True, result.get("error") + assert asyncio.run(config_tools.config_get_handler({"key": "llm.model"}))["value"] == "qwen3.8-max" + assert len(prompt.requests) == 1, "a config change must reach the human prompt" + assert str(prompt.requests[0].risk.level).endswith("HIGH") + + +def test_real_orchestrator_denial_blocks_the_write(cfg_home) -> None: + """A declined prompt must leave the setting untouched.""" + from leapflow.security.approval import ApprovalDecision + from leapflow.security.orchestrator import ApprovalOrchestrator + + class _AutoDeny: + async def request_approval(self, request): + return ApprovalDecision.DENY + + config_tools.set_config_approval_gate(ApprovalOrchestrator(_AutoDeny())) + + result = asyncio.run( + config_tools.config_set_handler({"key": "guardrail.enabled", "value": False}) + ) + + assert result["ok"] is False + assert _guardrail_is_on() + + +def test_a_broken_gate_denies_rather_than_opens(cfg_home) -> None: + """A gate that raises must not degrade into an open door.""" + + class _BrokenGate: + async def evaluate(self, action): + raise RuntimeError("gate exploded") + + config_tools.set_config_approval_gate(_BrokenGate()) + + result = asyncio.run( + config_tools.config_set_handler({"key": "guardrail.enabled", "value": False}) + ) + + assert result["ok"] is False + assert _guardrail_is_on(), "a gate error must not let the write through" + + +# ── The status bar must learn about the change ───────────────────────── + + +def test_stream_metadata_carries_the_active_model() -> None: + """A daemon-mode TUI caches the model at startup and only updates from this. + + Symptom this prevents: ``config_get`` reports the new model while the status + bar still shows the old one. The change notification in the chat loop is + change-detection based and a write has already refreshed the signature, so + the model has to travel on ordinary metadata instead. + """ + from types import SimpleNamespace + + from leapflow.daemon._service_helpers import engine_context_metadata + + metadata = engine_context_metadata( + None, SimpleNamespace(llm_context_length=1_000_000, llm_model="qwen3.8-max"), + ) + + assert metadata["llm_model"] == "qwen3.8-max" + assert metadata["llm_context_length"] == 1_000_000 + + +def test_stream_metadata_omits_an_empty_model() -> None: + """An unset model must not blank out whatever the bar already shows.""" + from types import SimpleNamespace + + from leapflow.daemon._service_helpers import engine_context_metadata + + metadata = engine_context_metadata(None, SimpleNamespace(llm_context_length=0, llm_model="")) + + assert "llm_model" not in metadata diff --git a/tests/test_context_disclosure.py b/tests/test_context_disclosure.py index 2dcf749..2dfe711 100644 --- a/tests/test_context_disclosure.py +++ b/tests/test_context_disclosure.py @@ -161,14 +161,40 @@ def test_hub_and_gateway_tools_are_explicitly_classified_as_heavy() -> None: assert manifests[name].is_core is False -def test_file_read_schema_discourages_workspace_config_probe() -> None: +def test_file_read_schema_redirects_config_reads_to_the_config_tools() -> None: + """The redirect must name the capability, not enumerate config paths. + + The previous wording forbade one specific path and then listed the real ones, + which both failed to stop the probing (the model simply tried another path) + and handed it fresh targets. With ``config_*`` tools available the guidance is + a pointer to them, and no LeapFlow path belongs in this description. + """ file_read_def = next( item for item in TOOL_DEFINITIONS if item.get("function", {}).get("name") == "file_read" ) description = str(file_read_def["function"].get("description", "")) - assert "Do not probe `/.leapflow/config.json`" in description - assert "~/.leapflow/config/user.yaml" in description - assert "~/.leapflow/profiles//config/*.yaml" in description - assert "/.leapflow/config.yaml" in description + for tool in ("config_list", "config_get", "config_set"): + assert tool in description + # No config path may be advertised here — that is what invited the probing. + for leaked in ("~/.leapflow", ".leapflow/config.json", "config/user.yaml", "profiles/"): + assert leaked not in description + + +def test_config_tools_are_core_and_writes_are_not() -> None: + """Reading settings must be always-available; writing must not be. + + A config read the model cannot see is the whole reason it fell back to file + probing, so ``config_get``/``config_list`` belong in the CORE floor. The + write is a mutation and stays behind progressive disclosure. + """ + plan = DisclosurePlanner().plan( + TOOL_DEFINITIONS, + DisclosureRuntimeState(native_tools_enabled=True), + ) + names = _tool_names(plan) + + assert "config_get" in names + assert "config_list" in names + assert "config_set" not in names