diff --git a/docs/agent.md b/docs/agent.md index e6cac8f..053929c 100644 --- a/docs/agent.md +++ b/docs/agent.md @@ -25,7 +25,7 @@ Forced context compression triggered by the `{"type":"compact"}` WebSocket control message (TUI `/compact`, `Ctrl+X C`). Bypasses the token threshold and runs the compressor immediately, emitting `CompressionStarted` + `ContextCompressed`. Raises `NothingToCompactError` (surfaced as an error frame) when there is nothing to compress. ### ContextVar restoration -`run_ephemeral` saves the parent's `current_session_id`, `current_model`, `current_working_directory`, `current_user_id`, `current_user_role`, and `current_user_info` before starting and restores them in a `finally` block. This prevents background tasks or the next parent iteration from inheriting stale subagent IDs. `run_stream` likewise sets and resets `current_working_directory` from the message `cwd`. +`run_ephemeral` saves the parent's `current_session_id`, `current_model`, `current_profile_id`, `current_working_directory`, `current_user_id`, `current_user_role`, and `current_user_info` before starting and restores them in a `finally` block. This prevents background tasks or the next parent iteration from inheriting stale subagent IDs. `run_stream` likewise sets and resets `current_working_directory` from the message `cwd`. --- diff --git a/docs/architecture.md b/docs/architecture.md index ee1b31a..bc9411a 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -66,13 +66,14 @@ Defined in `navi/tools/_internal/base.py`. -**Legacy path**: `ContextVar`s (`current_session_id`, `current_event_sink`, `current_stop_event`, `current_model`, `current_user_id`, `current_user_role`, `current_user_info`) are still present for backward compatibility. +**Legacy path**: `ContextVar`s (`current_session_id`, `current_event_sink`, `current_stop_event`, `current_model`, `current_profile_id`, `current_user_id`, `current_user_role`, `current_user_info`) are still present for backward compatibility. **Current path**: `Agent` builds a `ToolContext` dataclass explicitly and passes it into every tool's `execute()` call. This removes hidden dependencies and makes tool execution deterministic and testable: | Field | Type | Purpose | |---|---|---| | `session_id` | `str \| None` | Session ID for per-session state (SSH pool, scratchpad, todo) | +| `profile_id` | `str \| None` | Profile the run executes as (follows `switch_profile`); a subagent carries its own, not the parent's | | `event_sink` | `Queue \| None` | Queue where subagent events are written; parent drains it in real time | | `stop_event` | `Event \| None` | Cooperative stop signal checked before each LLM call | | `model` | `list[str] \| str \| None` | Current profile model — tools that call the LLM read this | diff --git a/docs/mechanics.md b/docs/mechanics.md index 8e055f9..54efc6a 100644 --- a/docs/mechanics.md +++ b/docs/mechanics.md @@ -60,7 +60,7 @@ | **Subagent planning phase** | Optionally runs the 2-phase planning pipeline (analysis + execution plan) before the subagent's tool loop; sub-agents execute without confirmation. | `profile.subagent_planning_enabled` | `subagent_runner.py` | ✅ | | **Parent session ID passthrough** | Sets session ContextVar to parent's ID so session-aware tools resolve paths correctly. | `parent_session_id` param | `agent.py` | ✅ | | **Dedicated subagent tool list** | Uses `profile.tools.subagent` if non-empty; falls back to `profile.tools.agent`. | `profile.tools.subagent` | `agent.py` | ✅ | -| **ContextVar restoration** | Saves/restores `current_session_id`, `current_model`, `current_user_id`, `current_user_role`, `current_user_info` in `finally` block. | None | `agent.py` | ✅ | +| **ContextVar restoration** | Saves/restores `current_session_id`, `current_model`, `current_profile_id`, `current_user_id`, `current_user_role`, `current_user_info` in `finally` block. | None | `agent.py` | ✅ | ## Planning Pipeline (`navi/core/planning.py`) @@ -177,7 +177,7 @@ | **ShareFileTool** | Copy file into session directory, return public download link. | `PUBLIC_URL`, `SESSION_FILES_DIR`, `SHARE_FILE_MAX_SIZE_MB` | `share_file.py` | ✅ | | **ContentPublishTool** | Register session file for inline viewing in chat. | `SESSION_FILES_DIR` | `content_publish.py` | ✅ | | **ToolManualTool** | Return `manuals/{tool}.md` or auto-generate from schema. | `MANUALS_DIR` | `tool_manual.py` | ✅ | -| **ListToolsTool** | Return tools enabled for a profile, including MCP group expansion. | `tools/enabled.json` | `list_tools.py` | ✅ | +| **ListToolsTool** | Tools a profile can actually call: live-registry truth, MCP group expansion, agent/subagent scope, defaults to the running profile. | `tools/enabled.json` | `list_tools.py` | ✅ | | **ReloadToolsTool** | Hot-reload user tools, context providers, and MCP servers without restart. | `settings.tools_dir`, `settings.context_providers_dir` | `reload_tools.py` | ✅ | | **ScheduleRecallTool** | Schedule headless callback (once/recurring/immediate). | None | `schedule_recall.py` | ✅ | | **ManageRecallTool** | Cancel/skip/list scheduled recalls. | None | `manage_recall.py` | ✅ | @@ -191,7 +191,7 @@ |---|---|---|---|---| | **`Tool` ABC** | Abstract base. Self-describes via `name`, `description`, `parameters`. | None | `base.py` | ✅ | | **`ToolResult`** | Standard return container: `success`, `output`, `error`, `metadata`. | None | `base.py` | ✅ | -| **ContextVars** | Async-safe vars set before each tool call: `current_session_id`, `current_event_sink`, `current_stop_event`, `current_model`, `current_user_id`, `current_user_role`, `current_user_info`. | None | `base.py` | ✅ | +| **ContextVars** | Async-safe vars set before each tool call: `current_session_id`, `current_event_sink`, `current_stop_event`, `current_model`, `current_profile_id`, `current_user_id`, `current_user_role`, `current_user_info`. | None | `base.py` | ✅ | | **Tool loader** | Discovers module-level (`name`, `description`, `parameters`, `execute`) and class-based tools. Errors isolated per file. | None | `loader.py` | ✅ | | **`ToolMiddleware`** | Pre/post execute hooks for logging/metrics/rate limiting. | None | `middleware.py` | ❌ | | **`LoggingMiddleware`** | Logs every tool execution with duration and result summary. | None | `logging_middleware.py` | ❌ | diff --git a/docs/tools.md b/docs/tools.md index 9110cb9..318a468 100644 --- a/docs/tools.md +++ b/docs/tools.md @@ -42,7 +42,7 @@ | `TodoTool` | `todo` | Per-session task checklist (set/update/read) | | `ScratchpadTool` | `scratchpad` | Per-session named working notes (write/append/read/clear) | | `ReloadToolsTool` | `reload_tools` | Hot-reload user tools and context providers without server restart | -| `ListToolsTool` | `list_tools` | Tools a profile can actually use, grouped by source (`native`, then one section per `mcp____`). Names only by default; `query` narrows by substring over names and descriptions, `verbose` adds descriptions. Declared-but-unregistered tools (server not connected) are listed separately as "Not registered" instead of being advertised | +| `ListToolsTool` | `list_tools` | Tools a profile can actually use, grouped by source (`native`, then one section per `mcp____`). Defaults to the profile the run executes as (`profile_id` overrides it); `scope` selects that profile's `agent` or `subagent` toolset. Names only by default; `query` narrows by substring over names and descriptions, `verbose` adds descriptions. Declared-but-unregistered tools (server not connected) are listed separately as "Not registered" instead of being advertised | | `ToolManualTool` | `tool_manual` | Return manuals/{name}.md or auto-generate from schema | | `MemoryTool` | `memory` | Unified memory tool: save, search, and forget facts | | `SpawnAgentTool` | `spawn_agent` | Spawn an isolated subagent (blocking). Optional `profile_id` selects another profile; omitted means parent profile. `inherit_system_prompt=true` prepends the parent profile's full system prompt as a base layer for the subagent | diff --git a/navi/core/agent.py b/navi/core/agent.py index 7317456..76f3ad2 100644 --- a/navi/core/agent.py +++ b/navi/core/agent.py @@ -359,11 +359,13 @@ from navi.tools._internal.base import ( current_session_id as _sid_var, current_model as _model_var, + current_profile_id as _profile_var, current_working_directory as _cwd_var, ) _sid_token = _sid_var.set(session_id) _model_var.set(profile.model) + _profile_var.set(profile.id) _cwd_token = _cwd_var.set(cwd) # Pre-turn compression: if the last turn filled the context past the @@ -672,6 +674,7 @@ tool_ctx = ToolContext( session_id=session_id, + profile_id=profile.id, event_sink=None, # set per-tool inside _execute_tools_with_sink stop_event=stop_event, model=profile.model, @@ -730,6 +733,9 @@ session.profile_id = fresh.profile_id profile = self._profiles.get(session.profile_id) self._set_active_profile(profile) + # The rest of this run executes as the new profile — tools that + # resolve a default profile must not keep reporting the old one. + _profile_var.set(profile.id) tools = self._tool_list(profile.get_agent_tools()) tool_schemas = [t.schema() for t in tools] llm = self._get_backend(profile.llm_backend) diff --git a/navi/core/subagent_runner.py b/navi/core/subagent_runner.py index 51c9691..65953a2 100644 --- a/navi/core/subagent_runner.py +++ b/navi/core/subagent_runner.py @@ -85,6 +85,7 @@ from navi.tools._internal.base import ( current_session_id as _sid_var, current_model as _model_var, + current_profile_id as _profile_var, current_todo_session_id as _todo_sid_var, current_user_id as _uid_var, current_user_role as _role_var, @@ -93,6 +94,7 @@ _prev_sid = _sid_var.get(None) _prev_model = _model_var.get(None) + _prev_profile = _profile_var.get(None) _prev_todo_sid = _todo_sid_var.get(None) _prev_uid = _uid_var.get(None) _prev_role = _role_var.get() @@ -107,6 +109,9 @@ profile = self._profiles.get(profile_id) _model_var.set(profile.model) + # The sub-agent's own profile, not the parent's: tools that default to + # "the profile I run as" must see the one this run was spawned with. + _profile_var.set(profile.id) exclude = set(exclude_tools or []) scope = profile.get_subagent_tools() @@ -346,6 +351,7 @@ tool_ctx = ToolContext( session_id=tool_session_id, + profile_id=profile.id, event_sink=sink, stop_event=stop_event, model=profile.model, @@ -460,6 +466,7 @@ finally: _sid_var.set(_prev_sid) _model_var.set(_prev_model) + _profile_var.set(_prev_profile) _todo_sid_var.set(_prev_todo_sid) _uid_var.set(_prev_uid) _role_var.set(_prev_role) diff --git a/navi/core/tasks.py b/navi/core/tasks.py index 0c385d3..04c2650 100644 --- a/navi/core/tasks.py +++ b/navi/core/tasks.py @@ -296,6 +296,7 @@ current_user_id, current_user_role, current_user_info, + current_profile_id, current_session_id, ) @@ -304,6 +305,7 @@ # the dead foreground sink, and (b) ignores the session's Stop signal. bg_ctx = ToolContext( session_id=getattr(ctx, "session_id", None), + profile_id=getattr(ctx, "profile_id", None), event_sink=job.ring, stop_event=job.stop_event, model=getattr(ctx, "model", None), @@ -322,6 +324,7 @@ (current_user_id, getattr(ctx, "user_id", None)), (current_user_role, getattr(ctx, "user_role", "user")), (current_user_info, getattr(ctx, "user_info", None)), + (current_profile_id, getattr(ctx, "profile_id", None)), ] for var, value in overrides: diff --git a/navi/tools/_internal/base.py b/navi/tools/_internal/base.py index a70ac9d..c35373d 100644 --- a/navi/tools/_internal/base.py +++ b/navi/tools/_internal/base.py @@ -43,6 +43,12 @@ # to tools that need to make their own LLM calls (e.g. AIHelper-powered tools). current_model: ContextVar[list[str] | str | None] = ContextVar("current_model", default=None) +# Set by run_stream() (and re-set when switch_profile reloads the run) / run_ephemeral() +# to expose which profile the current run is executing as. Tools that report or resolve +# per-profile configuration (list_tools) default to this when no profile is named. +# A sub-agent sets its own profile id, not the parent's. +current_profile_id: ContextVar[str | None] = ContextVar("current_profile_id", default=None) + # Set by run_stream() alongside current_user_id. Admins bypass sandbox restrictions. current_user_id: ContextVar[str | None] = ContextVar("current_user_id", default=None) @@ -75,6 +81,9 @@ """Explicit execution context passed to tools instead of hidden ContextVars.""" session_id: str | None = None + # Profile the run executes as — the one switch_profile last selected. Tools that + # need a profile when the caller named none fall back to this. + profile_id: str | None = None event_sink: asyncio.Queue | None = None stop_event: asyncio.Event | None = None model: list[str] | str | None = None diff --git a/navi/tools/list_tools.py b/navi/tools/list_tools.py index f280eb4..202e1a2 100644 --- a/navi/tools/list_tools.py +++ b/navi/tools/list_tools.py @@ -4,9 +4,11 @@ from navi.mcp.tools import build_mcp_name -from ._internal.base import Tool, ToolContext, ToolResult +from ._internal.base import Tool, ToolContext, ToolResult, current_profile_id NATIVE_SOURCE = "native" +SCOPE_AGENT = "agent" +SCOPE_SUBAGENT = "subagent" _MISSING_HINT = ( "Declared for this profile but not registered — their MCP server is not " "connected (or the tool is gone). Do not call these; pick another tool." @@ -17,16 +19,30 @@ name = "list_tools" description = ( "Returns the tools a profile can actually use, grouped by source: its built-in " - "tools as 'native' and each MCP server as 'mcp____'. Names only by " - "default — narrow it with query (substring over names and descriptions) or add " - "verbose for descriptions. For one tool's full docs use tool_manual." + "tools as 'native' and each MCP server as 'mcp____'. Defaults to the " + "profile you are running as — name another with profile_id. scope picks that " + "profile's agent or subagent toolset (spawn_agent runs the latter). Names only " + "by default — narrow it with query (substring over names and descriptions) or " + "add verbose for descriptions. For one tool's full docs use tool_manual." ) parameters = { "type": "object", "properties": { "profile_id": { "type": "string", - "description": "Profile ID. Returns only the tools enabled for this profile.", + "description": ( + "Profile ID to inspect. Omit to inspect the profile you are currently " + "running as." + ), + }, + "scope": { + "type": "string", + "enum": [SCOPE_AGENT, SCOPE_SUBAGENT], + "description": ( + "Which of that profile's two toolsets to list: 'agent' (what this " + "profile calls directly, the default) or 'subagent' (what a " + "spawn_agent sub-agent running this profile gets)." + ), }, "query": { "type": "string", @@ -44,7 +60,6 @@ ), }, }, - "required": ["profile_id"], } def __init__( @@ -61,12 +76,20 @@ if self._registry is None: return ToolResult(success=False, output="Registry not available.", error="no_registry") - profile_id = params.get("profile_id") + profile_id = (params.get("profile_id") or "").strip() + # No profile named: report on the one this run executes as, so the common + # question ("what do I have?") needs no argument the agent may not know. + defaulted = not profile_id + if defaulted: + profile_id = (ctx.profile_id if ctx else None) or current_profile_id.get() or "" if not profile_id: return ToolResult( success=False, - output="Missing required argument: profile_id. Specify a profile ID to see its enabled tools.", - error="missing_profile_id", + output=( + "No profile named and none is active in this context. Pass profile_id " + "explicitly — list_profiles shows the available IDs." + ), + error="no_active_profile", ) if self._profile_registry is None: @@ -81,10 +104,18 @@ success=False, output=f"Profile '{profile_id}' not found.", error="profile_not_found" ) + scope = (params.get("scope") or SCOPE_AGENT).strip().lower() + if scope not in (SCOPE_AGENT, SCOPE_SUBAGENT): + return ToolResult( + success=False, + output=f"Unknown scope '{scope}'. Use '{SCOPE_AGENT}' or '{SCOPE_SUBAGENT}'.", + error="unknown_scope", + ) + query = (params.get("query") or "").strip().lower() verbose = bool(params.get("verbose")) - declared = self._declared_by_source(profile) + declared = self._declared_by_source(profile, scope) declared_total = sum(len(names) for names in declared.values()) # Resolve against the LIVE registry, not the config: a tool the config @@ -111,7 +142,7 @@ if kept: available[source] = kept - lines = [self._header(profile_id, matched, declared_total, query)] + lines = [self._header(profile_id, matched, declared_total, query, scope, defaulted)] for source in self._ordered_sources(available): names = available[source] if verbose: @@ -127,16 +158,20 @@ if not query and not verbose: lines.append("") - lines.append("Pass query=… to filter, verbose=true for descriptions, tool_manual for one tool's docs.") + lines.append( + "Pass query=… to filter, verbose=true for descriptions, tool_manual for one tool's docs." + ) return ToolResult(success=True, output="\n".join(lines)) # ── helpers ────────────────────────────────────────────────────────── - def _declared_by_source(self, profile) -> dict[str, list[str]]: + def _declared_by_source(self, profile, scope: str = SCOPE_AGENT) -> dict[str, list[str]]: """Names the profile enables, bucketed by source ('native', 'mcp____').""" - scope = profile.get_agent_tools() - groups: dict[str, list[str]] = {NATIVE_SOURCE: list(scope.native)} + scope_config = ( + profile.get_subagent_tools() if scope == SCOPE_SUBAGENT else profile.get_agent_tools() + ) + groups: dict[str, list[str]] = {NATIVE_SOURCE: list(scope_config.native)} # Same reader the agent itself uses — a second, cwd-relative copy of # this path made list_tools disagree with the real toolset. @@ -146,8 +181,8 @@ if name not in groups[NATIVE_SOURCE]: groups[NATIVE_SOURCE].append(name) - if scope.mcp and self._mcp_manager: - for server_name, group_names in scope.mcp.items(): + if scope_config.mcp and self._mcp_manager: + for server_name, group_names in scope_config.mcp.items(): source = build_mcp_name(server_name, "") bucket = groups.setdefault(source, []) if "*" in group_names: @@ -176,10 +211,24 @@ return ([NATIVE_SOURCE] if NATIVE_SOURCE in available else []) + servers @staticmethod - def _header(profile_id: str, matched: int, declared_total: int, query: str) -> str: + def _header( + profile_id: str, + matched: int, + declared_total: int, + query: str, + scope: str, + defaulted: bool, + ) -> str: + # Say which profile was assumed and which toolset was read — an agent that + # asked about nobody must not read a subagent list as its own tools. + notes = [] + if defaulted: + notes.append("current profile") + if scope == SCOPE_SUBAGENT: + notes.append(f"{SCOPE_SUBAGENT} scope") + prefix = f"Tools for profile '{profile_id}'" + if notes: + prefix += f" ({', '.join(notes)})" if query: - return ( - f"Tools for profile '{profile_id}' matching '{query}': " - f"{matched} of {declared_total} declared." - ) - return f"Tools for profile '{profile_id}' ({matched} available):" + return f"{prefix} matching '{query}': {matched} of {declared_total} declared." + return f"{prefix} ({matched} available):" diff --git a/tests/unit/tools/test_list_tools.py b/tests/unit/tools/test_list_tools.py index 3515cd2..ad72f2b 100644 --- a/tests/unit/tools/test_list_tools.py +++ b/tests/unit/tools/test_list_tools.py @@ -12,6 +12,7 @@ from navi.core.registry import ProfileRegistry, ToolRegistry from navi.profiles.base import ToolConfig, ToolScopeConfig +from navi.tools._internal.base import ToolContext from navi.tools.list_tools import ListToolsTool from tests.conftest_factory import FakeTool, make_profile @@ -20,7 +21,7 @@ def no_user_enabled_tools(monkeypatch): """Every profile also picks up tools/enabled.json, a real file on the machine running the tests — tests must see exactly what they declare.""" - monkeypatch.setattr("navi.core.tool_utils.load_user_enabled_tools", lambda: []) + monkeypatch.setattr("navi.core.tool_utils.load_user_enabled_tools", list) class FakeMcpManager: @@ -33,13 +34,24 @@ return list(self._groups.get(server_name, {}).get(group_name, [])) -def make_profile_with(profile_id: str, native: list[str], mcp: dict | None = None): +def make_profile_with( + profile_id: str, + native: list[str], + mcp: dict | None = None, + subagent_native: list[str] | None = None, +): """A profile holding exactly the declared tools — no enabled_tools migration on top.""" + subagent = ( + ToolScopeConfig(native=list(subagent_native)) + if subagent_native is not None + else ToolScopeConfig() + ) return make_profile( profile_id, enabled_tools=[], tools=ToolConfig( agent=ToolScopeConfig(native=list(native), mcp=dict(mcp or {})), + subagent=subagent, ), ) @@ -191,11 +203,12 @@ @pytest.mark.asyncio -async def test_requires_a_profile_id(): +async def test_no_profile_in_context_is_an_error_that_says_what_to_do(): result = await make_tool(make_registry(), make_profiles()).execute({}) assert result.success is False - assert result.error == "missing_profile_id" + assert result.error == "no_active_profile" + assert "list_profiles" in result.output @pytest.mark.asyncio @@ -207,3 +220,69 @@ assert result.success is False assert result.error == "profile_not_found" + + +@pytest.mark.asyncio +async def test_defaults_to_the_profile_the_run_executes_as(): + """Omitted profile_id means "mine" — the agent answers "what do I have" with no argument.""" + profiles = make_profiles( + make_profile_with("secretary", ["todo"]), + make_profile_with("server_admin", ["ssh_exec"]), + ) + registry = make_registry(("todo", "notes"), ("ssh_exec", "remote")) + tool = make_tool(registry, profiles) + + result = await tool.execute({}, ctx=ToolContext(profile_id="server_admin")) + + assert "ssh_exec" in result.output + assert "todo" not in result.output + # Which profile was assumed, so the answer cannot be misread as another's. + assert "'server_admin' (current profile)" in result.output + + +@pytest.mark.asyncio +async def test_explicit_profile_id_beats_the_running_profile(): + profiles = make_profiles( + make_profile_with("secretary", ["todo"]), + make_profile_with("server_admin", ["ssh_exec"]), + ) + registry = make_registry(("todo", "notes"), ("ssh_exec", "remote")) + + result = await make_tool(registry, profiles).execute( + {"profile_id": "secretary"}, ctx=ToolContext(profile_id="server_admin") + ) + + assert "todo" in result.output + assert "ssh_exec" not in result.output + assert "current profile" not in result.output + + +@pytest.mark.asyncio +async def test_scope_subagent_lists_what_a_spawned_agent_would_get(): + profiles = make_profiles( + make_profile_with("server_admin", ["ssh_exec", "todo"], subagent_native=["ssh_exec"]) + ) + registry = make_registry(("ssh_exec", "remote"), ("todo", "planning")) + + agent_side = await make_tool(registry, profiles).execute({"profile_id": "server_admin"}) + subagent_side = await make_tool(registry, profiles).execute( + {"profile_id": "server_admin", "scope": "subagent"} + ) + + assert "todo" in agent_side.output + assert "todo" not in subagent_side.output + assert "ssh_exec" in subagent_side.output + assert "(subagent scope)" in subagent_side.output + + +@pytest.mark.asyncio +async def test_unknown_scope_is_rejected(): + profiles = make_profiles(make_profile_with("secretary", ["todo"])) + registry = make_registry(("todo", "notes")) + + result = await make_tool(registry, profiles).execute( + {"profile_id": "secretary", "scope": "worker"} + ) + + assert result.success is False + assert result.error == "unknown_scope"