diff --git a/emrg/protocol.py b/emrg/protocol.py index a903a33d..52feb3dd 100644 --- a/emrg/protocol.py +++ b/emrg/protocol.py @@ -45,6 +45,13 @@ class TaskRequest: # danger-full-access, current behavior). Set by task config, never by # the agent itself. sandbox: Optional[str] = None + # The tools this turn may use, by name (issue #2465). `None` means "the + # daemon's default set"; a list is a whitelist, intersected with the tools + # actually registered, so a name that is not registered is dropped rather + # than refused. An **empty** list is meaningful — it asks for no tools at + # all — which is why both the sender and the reader test `is not None` + # rather than the truthiness `sandbox` above is tested for. + tools: Optional[list[str]] = None def to_dict(self) -> dict: d = { @@ -59,6 +66,8 @@ def to_dict(self) -> dict: d["images"] = self.images if self.sandbox: d["sandbox"] = self.sandbox + if self.tools is not None: + d["tools"] = self.tools return d diff --git a/emrg/server/daemon.py b/emrg/server/daemon.py index 3a48b3f7..d6480dc3 100644 --- a/emrg/server/daemon.py +++ b/emrg/server/daemon.py @@ -178,6 +178,72 @@ def resolve_client_tier(session, req_sandbox: str | None = None) -> str: #: roots — and two hand-written unions would be two chances to drift apart. _ROOT_CONSUMING_TOOLS = SHELL_TOOL_NAMES | {"write", "edit"} +#: The tools a **host conversation** is offered, by name (issue #2465). +#: +#: This is a whitelist, and naming a tool is the only way to offer it: a tool +#: registered in ``build_tool_registry`` but named in neither this list nor +#: ``scheduled_task_tool_names()`` is invisible to every session. That is the +#: point — some tools are meaningful for one kind of turn and not another — but +#: it makes adding a tool a two-part act, and the second part is here. +#: +#: The shell entry is the platform's dialect rather than the word ``"bash"``: on +#: Windows the registered tool is ``pwsh``, so a list spelled ``"bash"`` would be +#: intersected away and the session would lose its shell silently. +#: +#: The platform is a parameter for the same reason it is one in +#: ``shell_tool_name``: the coupling that matters is that this set and the +#: registry it is intersected with were built for **one** platform, and a fact the +#: Windows CI leg alone can reach is a fact no host can check. Stating the platform +#: makes the pair askable about either dialect on any host. +def default_tool_names(platform_name: str | None = None) -> list[str]: + """The tool names a host conversation may use (issue #2465).""" + return [ + shell_tool_name(platform_name), "read", "write", "edit", "glob", "grep", + "submit_rant", "subagent", + ] + + +def scheduled_task_tool_names(platform_name: str | None = None) -> list[str]: + """The tool names a scheduled task's round may use (issue #2465). + + Today the same set a conversation gets. The split exists so that a tool + meant only for scheduled work (a review/owner role) can be added **here + alone** — no second mechanism inside the tool asking "who called me?". + """ + return default_tool_names(platform_name) + + +def effective_tool_names( + registry: ToolRegistry, requested: list[str] | None, + platform_name: str | None = None, +) -> list[str]: + """``(requested or the default set) ∩ registered``, in the caller's order. + + Intersection, not validation (issue #2465): a name that is not registered is + dropped in silence rather than refused, because a message naming a tool this + build does not carry is a message from a different build, and failing the + whole turn over it would make one stale name cost the round. Duplicates + collapse to their first occurrence, so the set cannot be handed the same tool + twice. + + ``platform_name`` is the platform ``registry`` was built for; it reaches + ``default_tool_names`` so that this intersection can be asked the question the + Windows runner asks, on any host. + """ + names = ( + default_tool_names(platform_name) if requested is None + else list(requested) + ) + seen: set[str] = set() + out: list[str] = [] + for name in names: + if name in seen: + continue + seen.add(name) + if registry.get(name) is not None: + out.append(name) + return out + #: How much of a child session's final answer a `subagent` call returns, in #: characters (issue #2451). The answer is the one thing the parent gets back, so it #: needs a ceiling — a child that dumps a whole file would spend the parent's context @@ -1649,6 +1715,11 @@ async def _handle_client(self, ws) -> None: timestamp=data.get("timestamp", ""), images=data.get("images"), sandbox=data.get("sandbox"), + # Issue #2465: which tools this turn may use, as the + # producer stated it. Absent (`None`) means the + # default set — which is what every client that does + # not care sends, and what a host conversation gets. + tools=data.get("tools"), ) except Exception as e: await self._send(ws, {"error": f"invalid task: {e}"}) @@ -4167,7 +4238,10 @@ async def _run_tool_loop_locked( }) @staticmethod - def _inject_tool_arguments(tc_name: str, args: dict, session, req: TaskRequest) -> None: + def _inject_tool_arguments( + tc_name: str, args: dict, session, req: TaskRequest, + tool_names: list[str] | None = None, + ) -> None: """Inject the parts of a tool call the model must not choose. Three rules. The first two are one fact — the session's cwd is where a @@ -4275,6 +4349,13 @@ def _inject_tool_arguments(tc_name: str, args: dict, session, req: TaskRequest) # a delegation cannot be the delegated. args["_parent_session_id"] = session.session_id args["_parent_cwd"] = str(session.cwd) + # The child inherits **this turn's** set (issue #2465). A delegation + # is the same authorisation seen one level down, so a round that may + # not use a tool cannot hand it to a session it spawns — otherwise + # `subagent` would be a way around the set rather than a use of it. + args["_parent_tools"] = ( + list(tool_names) if tool_names is not None else None + ) async def _apply_escalation( self, tc_name: str, args: dict, session, req: TaskRequest, @@ -4416,7 +4497,15 @@ async def _run_tool_loop( # The message was just built and persisted above; the same one goes in, # so the stored bytes are the sent bytes (issue #2311). self._inject_context_message(session, messages, context_message) - tools_base = self.tools.to_openai_tools() if allow_tools else [] + # Issue #2465: the tools this turn may actually use. Computed once and + # read by all three enforcement points — the table the model is offered, + # the lookup that runs a named tool, and the memory-reflection loop — + # because a filter at only one of them is a gate in name only. + turn_tool_names = effective_tool_names(self.tools, req.tools) + turn_tool_set = set(turn_tool_names) + tools_base = ( + self.tools.to_openai_tools(turn_tool_names) if allow_tools else [] + ) # P1 (rant 21:55:37): injection rounds do NOT consume the round budget; # force_ask latches "an Ask message was injected outside the round-top # injection (stop / Case 3 / loop-end)" so the next round uses an empty @@ -4472,7 +4561,9 @@ async def _run_tool_loop( }) # Fire-and-forget: reflect on whether to save memories - self._maybe_reflect_memory(session, req.prompt, full_content) + self._maybe_reflect_memory( + session, req.prompt, full_content, turn_tool_names, + ) return tools_openai = tools_base @@ -4906,7 +4997,9 @@ async def _run_tool_loop( }) # Fire-and-forget: reflect on whether to save memories - self._maybe_reflect_memory(session, req.prompt, full_content) + self._maybe_reflect_memory( + session, req.prompt, full_content, turn_tool_names, + ) # Issue #1114: a round GENUINELY completed here (final text # answer, no more tool calls) — stamp the completed-round file # so the staleness alarm can answer "how long since a @@ -5002,13 +5095,26 @@ async def _run_tool_loop( }) # The pieces of a call the model must not choose (D1). - self._inject_tool_arguments(tc_name, args, session, req) + self._inject_tool_arguments( + tc_name, args, session, req, turn_tool_names, + ) + # Issue #2465: a name outside this turn's set is answered + # exactly as a name that was never registered — the refusal + # below, not a second vocabulary for saying no. Decided + # **before** the escalation path, because escalation is a + # question put to the host: a call that cannot run must not + # ask permission to run first, and an answer of "yes" would + # then be reported as the call's outcome although the tool it + # widened for was never dispatched. + tool = ( + self.tools.get(tc_name) if tc_name in turn_tool_set else None + ) # Execute - refusal = await self._apply_escalation( - tc_name, args, session, req, + refusal = ( + await self._apply_escalation(tc_name, args, session, req) + if tool is not None else None ) - tool = self.tools.get(tc_name) if refusal is not None: # A refused escalation is the call's outcome, and the # command does not run at all: a hop that was not granted @@ -5036,7 +5142,9 @@ async def _run_tool_loop( result = ToolResult( tool_call_id=tc_id, name=tc_name, - content=f"Unknown tool: {tc_name}. Available: {self.tools.names}", + content=( + f"Unknown tool: {tc_name}. Available: {turn_tool_names}" + ), error=True, ) @@ -5123,7 +5231,9 @@ async def _run_tool_loop( }) # Fire-and-forget: reflect on whether to save memories - self._maybe_reflect_memory(session, req.prompt, full_content) + self._maybe_reflect_memory( + session, req.prompt, full_content, turn_tool_names, + ) # Issue #1114: max-tokens/other-stop with the final answer # persisted + broadcast = a genuinely completed round — stamp the # completed-round file (see Case 1; NOT done on the cancellation / @@ -7490,7 +7600,7 @@ async def _await_memory_reflection(self, session_id: str) -> None: async def _run_subagent_tool( self, *, parent_session_id: str, parent_cwd: str, prompt: str, - timeout_s: int, intent: str = "", + timeout_s: int, intent: str = "", parent_tools: list[str] | None = None, ) -> dict: """Run one child session for one `subagent` call. Answers text or an error. @@ -7514,6 +7624,7 @@ async def _run_subagent_tool( prompt=prompt, intent=intent, state=state, + parent_tools=parent_tools, )) try: done, _ = await asyncio.wait({child_task}, timeout=timeout_s) @@ -7544,7 +7655,7 @@ async def _run_subagent_tool( async def _run_subagent_child( self, *, parent_session_id: str, parent_cwd: str, prompt: str, - intent: str, state: dict, + intent: str, state: dict, parent_tools: list[str] | None = None, ) -> dict: """The body of one delegation: pick a slot, run the child, report its answer.""" started = time.monotonic() @@ -7616,6 +7727,10 @@ async def _run_subagent_child( prompt=prompt, timestamp="", sandbox=parent.sandbox, + # Issue #2465: the child's tool set is the parent's — inherited + # the same way its tier is, because authorisation a caller can + # choose for the thing it is authorising is not a boundary. + tools=parent_tools, ) assistants_before = sum( 1 for r in child._read_history() if r.get("role") == "assistant" @@ -7685,7 +7800,8 @@ def _subagent_report( ) def _maybe_reflect_memory( - self, session: Session, user_prompt: str, assistant_content: str + self, session: Session, user_prompt: str, assistant_content: str, + tool_names: list[str] | None = None, ) -> None: """Fire-and-forget: ask LLM to reflect on the exchange and save memories. @@ -7818,8 +7934,21 @@ async def _reflect(): "directory to keep the index in sync." ) - # Run a mini tool loop: LLM can create/update memory files via tools - tools = self.tools.to_openai_tools() + # Run a mini tool loop: LLM can create/update memory files via tools. + # Issue #2465: this loop builds its **own** table, so a filter in + # `_run_tool_loop` alone would leave this copy offering every + # registered tool — the third enforcement point, not a duplicate. + # + # `None` is "the caller stated no set", and the answer to that is + # the **default set**, not the whole registry. Answering the same + # silence two ways is a gate with a hole in it: a tool registered + # but named in neither whitelist (the review/owner role this issue + # leaves room for) would be offered here while every other site + # refuses it. One silence, one meaning — and the meaning is the + # function that already states it. + reflect_names = effective_tool_names(self.tools, tool_names) + tools = self.tools.to_openai_tools(reflect_names) + reflect_tool_set = set(reflect_names) messages: list[dict] = [{"role": "user", "content": prompt}] max_rounds = 4 @@ -7880,7 +8009,10 @@ async def _reflect(): except json.JSONDecodeError: args = {} - tool = self.tools.get(tc_name) + tool = ( + self.tools.get(tc_name) + if tc_name in reflect_tool_set else None + ) if tool: try: result = await tool.execute(args) diff --git a/emrg/server/scheduler.py b/emrg/server/scheduler.py index 3745b315..8a4a9891 100644 --- a/emrg/server/scheduler.py +++ b/emrg/server/scheduler.py @@ -2678,6 +2678,12 @@ async def _run_evolution_cycle(self) -> str: # silent ending (see `_end_heartbeat`). return self._NOT_STARTED + # Issue #2465: this round's tool set is the **scheduled-task** one. + # Imported here rather than at module scope because `emrg.server.daemon` + # imports this module; the whitelists live there, beside the registration + # they describe, so there is one place that says what a tool set is. + from emrg.server.daemon import scheduled_task_tool_names + task_msg = json.dumps( { "type": "task", @@ -2704,6 +2710,11 @@ async def _run_evolution_cycle(self) -> str: # forces, not what dirt does. `EMRG_TASK_DIRTY_OVERRIDE` (audited # receipt) skips that convergence rather than a downgrade. "sandbox": await self._effective_sandbox(), + # Issue #2465: which tools this round may use. Named by the + # producer because only the producer knows this is a scheduled + # round rather than something the host typed — a fact the daemon + # would otherwise have to guess from the session's name. + "tools": scheduled_task_tool_names(), }, ensure_ascii=False, ) diff --git a/emrg/tools/registry.py b/emrg/tools/registry.py index 6775fd78..7cb80259 100644 --- a/emrg/tools/registry.py +++ b/emrg/tools/registry.py @@ -27,16 +27,25 @@ def get(self, name: str) -> Optional[ToolExecutor]: """Look up a tool by name. Returns None if not found.""" return self._tools.get(name) - def to_openai_tools(self) -> list[dict]: - """Serialize all registered tools to OpenAI function-calling format. + def to_openai_tools(self, names: list[str] | None = None) -> list[dict]: + """Serialize registered tools to OpenAI function-calling format. + + :param names: the only tool names to serialize, in registry order. + ``None`` (the default) serializes the whole registry, which is what + a caller that is not narrowing a turn's set wants. A name this + registry does not hold is simply not there — the list narrows, it + does not create (issue #2465). Returns a list of dicts suitable for the `tools` field of a chat completions API request: [{"type": "function", "function": {"name": ..., "description": ..., "parameters": ...}}] """ + allowed = None if names is None else set(names) result: list[dict] = [] for tool in self._tools.values(): d = tool.definition() + if allowed is not None and d.name not in allowed: + continue result.append({ "type": "function", "function": { diff --git a/emrg/tools/subagent_tool.py b/emrg/tools/subagent_tool.py index c4b84556..48f9b236 100644 --- a/emrg/tools/subagent_tool.py +++ b/emrg/tools/subagent_tool.py @@ -42,8 +42,9 @@ from emrg.tools.base import ToolExecutor #: The runner the daemon supplies: ``await runner(parent_session_id=…, -#: parent_cwd=…, prompt=…, timeout_s=…, intent=…)`` answering either -#: ``{"text": …}`` or ``{"error": …}``. +#: parent_cwd=…, prompt=…, timeout_s=…, intent=…, parent_tools=…)`` answering +#: either ``{"text": …}`` or ``{"error": …}``. ``parent_tools`` is the parent +#: turn's effective tool set (issue #2465), which the child inherits. Runner = Callable[..., Awaitable[dict]] #: A default that is generous enough for a real job and short enough that a @@ -165,12 +166,23 @@ async def execute(self, arguments: dict) -> ToolResult: error=True, ) + # The parent turn's tool set, injected by the daemon (issue #2465). A + # value that is not a list is treated as "no statement", which the + # daemon reads as its default set — the same meaning the message field + # has, so the two cannot disagree about silence. + raw_parent_tools = arguments.get("_parent_tools") + parent_tools = ( + [str(name) for name in raw_parent_tools] + if isinstance(raw_parent_tools, list) else None + ) + outcome = await self._runner( parent_session_id=str(parent_session_id), parent_cwd=str(parent_cwd), prompt=prompt, timeout_s=int(timeout_s), intent=str(arguments.get("intent") or ""), + parent_tools=parent_tools, ) if outcome.get("error"): return ToolResult(name="subagent", content=outcome["error"], error=True) diff --git a/tests/test_sandbox_escalation.py b/tests/test_sandbox_escalation.py index 2bb6ceec..7b63e5c5 100644 --- a/tests/test_sandbox_escalation.py +++ b/tests/test_sandbox_escalation.py @@ -38,6 +38,7 @@ from emrg.server.daemon import EmrgServer from emrg.server.tool_types import ToolDefinition, ToolResult from emrg.session import Session +from emrg.tools.shell_dialects import shell_tool_name # ── fixtures ──────────────────────────────────────────────────────────────── @@ -620,13 +621,20 @@ async def scenario(): def _drive_shell_call( tmp_path, monkeypatch, *, args: dict, answer=object(), sandbox="read-only", - tool_name="bash", + tool_name=None, ): """Run one tool loop whose single tool call carries `args`. Returns `(tool, frames, ws)`: the stub tool the loop executed (or not), everything it broadcast, and the subscriber that received it. + + `tool_name` defaults to the dialect **this platform mounts**, not to the word + `"bash"`: the turn's set is the default set, which names that dialect, so a + stand-in registered as the other one is refused before any of this file's + subject is reached — on a Windows runner the loop answers `Unknown tool: bash` + and every assertion here reads a call that never happened. """ + tool_name = tool_name or shell_tool_name() monkeypatch.setattr(daemon_mod, "_PLANTED_FIRE_MARKER_PATH", tmp_path / "planted-fire-heartbeat") monkeypatch.setattr(daemon_mod, "_PLANTED_FIRE_ROUND_COMPLETE_PATH", @@ -802,8 +810,9 @@ def test_the_widening_belongs_to_one_call_and_not_to_the_next(tmp_path, monkeypa server = _server() session = Session.create_with_id("esc-once", tmp_path) ws = _subscribe(server, session) - stub = _StubTool("bash") - server.tools._tools["bash"] = stub + shell = shell_tool_name() + stub = _StubTool(shell) + server.tools._tools[shell] = stub rounds = [ [{"command": "npm install", "sandbox_permissions": "danger-full-access", @@ -823,7 +832,8 @@ async def fake_stream(messages, tools=None): "usage": {"prompt_tokens": 10, "completion_tokens": 1}, "tool_calls": [{ "index": 0, "id": f"call_{n}", "type": "function", - "function": {"name": "bash", "arguments": json.dumps(rounds[n][0])}, + "function": {"name": shell, + "arguments": json.dumps(rounds[n][0])}, }], } else: diff --git a/tests/test_scheduler.py b/tests/test_scheduler.py index e8d68f55..3d46c46b 100644 --- a/tests/test_scheduler.py +++ b/tests/test_scheduler.py @@ -1869,6 +1869,16 @@ async def _fake_connect(): "the scheduled task message no longer says its prompt is not host input — the " "daemon will file one task prompt per cycle into the session's recall list" ) + # Issue #2465: the producer states which tool set this round may use. The + # daemon cannot tell a scheduled round from a typed one by the session's name, + # so the round that knows has to say it — and a producer that stops saying it + # silently widens every scheduled round to the default set. + from emrg.server.daemon import scheduled_task_tool_names + + assert msg.get("tools") == scheduled_task_tool_names(), ( + "the scheduled task message no longer names its tool set, so the round runs " + "with the default one" + ) # And the field is not there at the cost of the message: these are what the daemon # needs to run the turn at all. for key in ("session_id", "cwd", "prompt", "timestamp"): diff --git a/tests/test_tool_set_per_message.py b/tests/test_tool_set_per_message.py new file mode 100644 index 00000000..3af0b95c --- /dev/null +++ b/tests/test_tool_set_per_message.py @@ -0,0 +1,546 @@ +"""A message says which tools its turn may use, and the daemon enforces it (issue #2465). + +Before this, the tool table was fixed at daemon start: `build_tool_registry` +registered eight tools and nothing afterwards narrowed that set for any session or +any message, so a host conversation and a scheduled task's round were served +exactly the same list with no way to make them differ. + +The subjects here are the sites a filter has to reach for the set to be real, and +none of them proves another: + +* **the effective set** — four cases, because the intersection has four outcomes + and a test of the happy one says nothing about the silent drop; +* **the table offered to the model**, read off the wire: the `tools` argument the + loop actually hands the LLM, not a helper's return value; +* **the execution side**, driven for real: a registered tool that the turn's set + omits must not run — with a **positive control** that runs it when the set + admits it, so a red arm cannot be a harness that never executes anything; +* **the order against the approval channel**: a name the set omits must not put a + question to the host first, since escalation asks permission for a command — + with its own control, the same hop on a set that admits the shell; +* **the memory-reflection loop**, which builds its own table and therefore needs + its own assertion: a filter in `_run_tool_loop` alone leaves this copy whole, + and "no set stated" has to mean the default set here as it does everywhere; +* **the two ends of the message** — what a producer states and what the receiver + reads back. + +No daemon is started, stopped or restarted: `_run_tool_loop`, `_process_message` +and `_maybe_reflect_memory` are called directly, with the LLM, the broadcast and +the websocket stubbed. +""" + +from __future__ import annotations + +import asyncio +import json +import sys + +from emrg.config import LlmConfig +from emrg.protocol import TaskRequest +from emrg.server import daemon as daemon_mod +from emrg.server.daemon import ( + EmrgServer, + build_tool_registry, + default_tool_names, + effective_tool_names, + scheduled_task_tool_names, +) +from emrg.server.tool_types import ToolDefinition, ToolResult +from emrg.session import Session +from emrg.tools.shell_dialects import SHELL_TOOL_NAMES, shell_tool_name + + +def _registry(platform_name: str | None = None): + """The real registry, for a named platform or (by default) for this host. + + The platform defaults to the host rather than to a literal. A helper spelling + `"darwin"` mounts `bash` on a Windows runner while `default_tool_names()` + answers `pwsh`, so the shell is intersected away and every assertion about the + set ends up describing a platform the runner is not — which is how the first + version of this file read green on one leg and red on the other. + """ + return build_tool_registry(None, None, platform_name=platform_name) + + +def _server(tmp_path) -> EmrgServer: + server = EmrgServer(LlmConfig(base_url="http://localhost", api_key="test")) + server._projects_log = tmp_path / "projects.yml" + return server + + +class _StubTool: + """Records that it ran, so "did not run" is observable rather than inferred.""" + + def __init__(self) -> None: + self.calls: list[dict] = [] + + async def execute(self, args): + self.calls.append(dict(args)) + return ToolResult(content="the stub tool ran") + + +def _stub_two_tools(server: EmrgServer) -> _StubTool: + """Make `read` and `write` answer from one stub, and leave the rest real.""" + stub = _StubTool() + real_get = server.tools.get + server.tools.get = lambda name: stub if name in {"read", "write"} else real_get(name) + return stub + + +# ── the protocol field ──────────────────────────────────────────────── + + +def test_a_task_request_carries_its_tool_list(): + assert TaskRequest(tools=["read"]).to_dict()["tools"] == ["read"] + + +def test_an_empty_tool_list_is_a_statement_and_survives_the_wire(): + # `sandbox` is sent only when truthy; `tools` is sent when it is not None, + # because the empty list means "no tools at all" and dropping it would ask + # for the default set instead — the opposite of what it says. + assert TaskRequest(tools=[]).to_dict()["tools"] == [] + + +def test_an_absent_tool_list_stays_absent(): + assert "tools" not in TaskRequest().to_dict() + + +# ── the effective set: four outcomes ────────────────────────────────── + + +def test_a_list_is_honoured_by_intersection(): + assert effective_tool_names(_registry(), ["read", "grep"]) == ["read", "grep"] + + +def test_no_list_falls_back_to_the_default_set(): + registry = _registry() + assert effective_tool_names(registry, None) == [ + name for name in default_tool_names() if registry.get(name) is not None + ] + + +def test_an_unregistered_name_is_dropped_and_the_rest_are_kept(): + assert effective_tool_names(_registry(), ["read", "no-such-tool", "grep"]) == [ + "read", "grep", + ] + + +def test_a_list_of_nothing_but_unknown_names_yields_no_tools(): + assert effective_tool_names(_registry(), ["no-such-tool", "nor-this"]) == [] + + +def test_the_default_set_names_the_shell_the_registry_mounts(): + # The whitelist is written once and runs everywhere, so its shell entry must + # come from the platform rather than from the word "bash" — asked here in both + # dialects on whichever host runs this, plus the host's own. A reading taken + # only where the two dialects agree (every POSIX box) cannot see the failure at + # all, and one taken only on the Windows leg cannot be checked before pushing. + for platform_name in ("darwin", "win32", None): + subject = platform_name or sys.platform + registry = _registry(platform_name) + shells = {name for name in registry.names if name in SHELL_TOOL_NAMES} + assert shells == {shell_tool_name(platform_name)}, ( + f"{subject}: the registry mounted {shells}, not this platform's dialect" + ) + assert shells <= set(default_tool_names(platform_name)), ( + f"{subject}: the default set {default_tool_names(platform_name)} does not " + f"name {shells}, the only shell this registry carries" + ) + assert shells <= set(effective_tool_names(registry, None, platform_name)), ( + f"{subject}: the intersection " + f"{effective_tool_names(registry, None, platform_name)} dropped the shell, " + "so the session loses it without a word" + ) + + +def test_the_two_whitelists_name_tools_that_exist(): + registry = _registry() + for name in set(default_tool_names()) | set(scheduled_task_tool_names()): + if name in SHELL_TOOL_NAMES: + continue + assert registry.get(name) is not None, ( + f"{name!r} is in a whitelist but not registered — the whitelist and the " + "registration have drifted apart" + ) + + +# ── the table offered to the model, and the execution side ──────────── + + +def _drive_tool_loop(tmp_path, monkeypatch, name, requested, call_name, + arguments='{"intent": "x"}', sandbox=None, prepare=None): + """Run one turn whose first round calls `call_name`; report what it was offered.""" + monkeypatch.setattr(daemon_mod, "_PLANTED_FIRE_MARKER_PATH", tmp_path / "fire") + monkeypatch.setattr(daemon_mod, "_PLANTED_FIRE_ROUND_COMPLETE_PATH", tmp_path / "rc") + server = _server(tmp_path) + session = Session.create_with_id(f"tool-set-{name}", tmp_path) + stub = _stub_two_tools(server) + # The reflection loop is a subject of its own below; here it would only add a + # background turn whose LLM is this test's stub. + server._maybe_reflect_memory = lambda *a, **k: None + if prepare is not None: + # Lets a test stub what the escalation path would ask (`_approval_channel`) + # before the turn runs, without widening this harness's return value. + prepare(server) + + offered: dict = {"tools": None} + state = {"round": 0} + + async def fake_stream(messages, tools=None): + state["round"] += 1 + if state["round"] == 1: + offered["tools"] = [ + entry["function"]["name"] for entry in (tools or []) + ] + yield { + "content": None, + "tool_calls": [{ + "index": 0, + "id": "call_0", + "function": {"name": call_name, "arguments": arguments}, + }], + "finish_reason": "tool_calls", + "usage": None, + } + else: + yield { + "content": "done", "tool_calls": None, "finish_reason": "stop", + "usage": {"prompt_tokens": 1, "completion_tokens": 1}, + } + + frames: list[dict] = [] + + async def fake_broadcast(session_id, payload): + frames.append(payload) + + server.llm.chat_stream = fake_stream + server._broadcast = fake_broadcast + req = TaskRequest( + id=f"req-{name}", session_id=session.session_id, prompt="go", + tools=requested, sandbox=sandbox, + ) + asyncio.run(server._run_tool_loop(req, None, session)) + return offered["tools"], stub.calls, frames + + +def test_the_model_is_offered_only_the_tools_the_message_named(tmp_path, monkeypatch): + offered, _calls, _frames = _drive_tool_loop( + tmp_path, monkeypatch, "named", ["read"], "read", + ) + assert offered == ["read"], ( + "the turn's set is the message's list; offering the whole registry here is " + "the defect the set exists to remove" + ) + + +def test_the_model_is_offered_the_default_set_when_the_message_names_none( + tmp_path, monkeypatch, +): + offered, _calls, _frames = _drive_tool_loop( + tmp_path, monkeypatch, "default", None, "read", + ) + assert offered == effective_tool_names(_registry(), None) + + +def test_a_registered_tool_outside_the_set_does_not_run(tmp_path, monkeypatch): + # `write` is registered, and this turn's set names only `read`. + offered, calls, frames = _drive_tool_loop( + tmp_path, monkeypatch, "refused", ["read"], "write", + ) + assert offered == ["read"] + assert calls == [], ( + "a tool the turn was never offered ran anyway — the serialised table is then " + "advice, not a gate" + ) + said = json.dumps([frame for frame in frames], ensure_ascii=False) + assert "unknown" in said.lower(), ( + "the call has to be answered, not dropped in silence: the model has to learn " + f"that the name is not available; frames said {said[:400]!r}" + ) + # The refusal names what this turn *does* have. Listing the whole registry + # would advertise, in the one message the model reads about availability, the + # tools the turn cannot use — the same leak as an unfiltered table, spoken. + assert f"Available: {['read']}" in said, ( + "the refusal has to name the turn's own set; frames said " + f"{said[:400]!r}" + ) + + +def test_a_tool_inside_the_set_does_run(tmp_path, monkeypatch): + # The positive control. Without it, the arm above would also pass on a harness + # that never executes any tool at all — "not caught" and "never ran" print the + # same green. + _offered, calls, _frames = _drive_tool_loop( + tmp_path, monkeypatch, "allowed", ["read", "write"], "write", + ) + assert calls, "the harness cannot execute a tool at all, so the refusal above proves nothing" + + +# ── the set decides before the host is asked anything ───────────────── + + +def _record_approval_channel(asked): + """A `_approval_channel` stand-in that records each question, then refuses. + + Refusing keeps the harness from running a command: the subject here is that + the host is asked at all, and a green assertion must not depend on what the + answer would have been. + """ + async def channel(session_id): + asked.append(session_id) + + async def ask(question): + return None + return ask + return channel + + +def _hop_arguments(): + """A shell call that asks for one hop — legal from `read-only` in one step.""" + return json.dumps({ + "intent": "x", + "sandbox_permissions": "workspace-write", + "justification": "the command needs to write inside the workspace", + }) + + +def test_a_name_outside_the_set_asks_the_host_nothing(tmp_path, monkeypatch): + # `read-only` is a tier the shell tool may widen from, so this call would + # reach the approval channel if the set were consulted afterwards. It is the + # order that is under test: an escalation for a tool the turn was never + # offered is a question about a command that cannot run either way. + asked: list[str] = [] + shell = shell_tool_name() + _offered, calls, frames = _drive_tool_loop( + tmp_path, monkeypatch, "no-approval", ["read"], shell, + arguments=_hop_arguments(), sandbox="read-only", + prepare=lambda server: setattr( + server, "_approval_channel", _record_approval_channel(asked), + ), + ) + assert asked == [], ( + "the host was asked to approve an escalation for a tool this turn was " + "never offered — the question goes out before the answer that the call " + "cannot run, and a yes would be reported as the call's outcome" + ) + assert calls == [] + said = json.dumps(frames, ensure_ascii=False) + assert "unknown" in said.lower(), ( + "the call still has to be answered as unknown, not dropped in silence; " + f"frames said {said[:400]!r}" + ) + + +def test_a_name_inside_the_set_does_ask_the_host(tmp_path, monkeypatch): + # The positive control for the arm above: same hop, same arguments, and the + # shell named in the set. Without it an `asked == []` assertion would also + # hold on a harness that can never reach the approval channel. + asked: list[str] = [] + shell = shell_tool_name() + _drive_tool_loop( + tmp_path, monkeypatch, "approval", ["read", shell], shell, + arguments=_hop_arguments(), sandbox="read-only", + prepare=lambda server: setattr( + server, "_approval_channel", _record_approval_channel(asked), + ), + ) + assert asked, ( + "the stub cannot observe an approval question at all, so the arm above " + "proves nothing about whether one is skipped" + ) + + +# ── the memory-reflection loop, which builds its own table ──────────── + + +def test_the_memory_reflection_loop_honours_the_same_set(tmp_path): + async def _test(): + server = _server(tmp_path) + session = Session.create_with_id("tool-set-reflect", tmp_path) + stub = _stub_two_tools(server) + tables: list[list[str]] = [] + rounds = {"n": 0} + + async def fake_chat(messages, tools=None): + rounds["n"] += 1 + if rounds["n"] == 1: + tables.append([entry["function"]["name"] for entry in (tools or [])]) + return { + "content": "", + "tool_calls": [{ + "id": "c1", "type": "function", + "function": {"name": "write", "arguments": "{}"}, + }], + } + return {"content": "no new memories", "tool_calls": None} + + server.llm.chat = fake_chat + server._maybe_reflect_memory( + session, + "a prompt long enough to be worth reflecting on " * 2, + "an answer long enough to be worth reflecting on " * 2, + ["read"], + ) + await asyncio.sleep(0.2) + assert tables, "the reflection loop never ran, so it asserted nothing" + assert tables[0] == ["read"], ( + "the reflection loop offers the whole registry while the turn offered one " + "tool — the second table is a second gate, and it is the one that is open" + ) + assert stub.calls == [], ( + "the reflection loop executed a tool the turn's set does not contain" + ) + + asyncio.run(_test()) + + +class _UnlistedTool: + """A registered tool no whitelist names — the shape a `boss`-style role has.""" + + def __init__(self, name: str) -> None: + self._name = name + self.calls: list[dict] = [] + + def definition(self) -> ToolDefinition: + return ToolDefinition( + name=self._name, + description="registered but named in neither whitelist", + parameters={"type": "object", "properties": {}}, + ) + + async def execute(self, args): + self.calls.append(dict(args)) + return ToolResult(content="the unlisted tool ran") + + +def test_the_reflection_loop_reads_silence_as_the_default_set(tmp_path): + """`None` means "no set stated", and the answer to that is the default set. + + Reading the same silence as the **whole registry** is a second meaning for + one fact, and it opens exactly the hole the whitelists close: a tool + registered but named nowhere would be offered by this loop while every other + turn refuses it. + """ + async def _test(): + server = _server(tmp_path) + session = Session.create_with_id("tool-set-reflect-silence", tmp_path) + unlisted = _UnlistedTool("unlisted-role") + server.tools.register(unlisted) + tables: list[list[str]] = [] + + async def fake_chat(messages, tools=None): + tables.append([entry["function"]["name"] for entry in (tools or [])]) + return {"content": "no new memories", "tool_calls": None} + + server.llm.chat = fake_chat + server._maybe_reflect_memory( + session, + "a prompt long enough to be worth reflecting on " * 2, + "an answer long enough to be worth reflecting on " * 2, + ) + await asyncio.sleep(0.2) + assert tables, "the reflection loop never ran, so it asserted nothing" + assert "unlisted-role" not in tables[0], ( + "a tool registered but named in no whitelist was offered by the " + "reflection loop — one silence, two meanings" + ) + assert unlisted.calls == [] + assert tables[0] == effective_tool_names(server.tools, None) + + asyncio.run(_test()) + + +# ── the message's two ends ──────────────────────────────────────────── + + +class _FakeSocket: + """A websocket stand-in the read loop can drive: `recv` yields frames, then ends.""" + + def __init__(self, frames: list[dict]) -> None: + self._frames = [json.dumps(frame) for frame in frames] + self.sent: list[dict] = [] + + async def recv(self) -> str: + if not self._frames: + from websockets.exceptions import ConnectionClosed + + raise ConnectionClosed(None, None) + return self._frames.pop(0) + + async def send(self, data) -> None: + self.sent.append(json.loads(data)) + # Yield to the loop. The turn is started with `asyncio.create_task`, and a + # disconnect cancels it in `_handle_client`'s `finally` — so a stand-in + # whose `send` never yields lets the connection close before the spawned + # turn has run at all, and the assertion below would read "not started" as + # "field dropped". + await asyncio.sleep(0) + + async def close(self) -> None: + pass + + +def test_the_task_message_carries_its_tool_list_into_the_turn(tmp_path, monkeypatch): + """Driven through `_handle_client`, because that is where a `task` is dispatched. + + `_process_message` has no `task` branch at all — it answers `unknown message + type` — so a test that hands a `task` frame to it certifies a door production + does not use. + + `HOME` and `TMPDIR` are pinned **for this test alone**: a `task` frame creates + a session, and creating one writes the machine-wide session index under the + home directory. Pinning them for the whole suite would turn every unrelated + test red (a home under a temporary root is itself a writable area); pinning + them here keeps this test's writes inside the test's own directory. + """ + monkeypatch.setenv("HOME", str(tmp_path / "home")) + monkeypatch.setenv("TMPDIR", str(tmp_path / "tmpdir")) + (tmp_path / "home").mkdir() + (tmp_path / "tmpdir").mkdir() + monkeypatch.setattr(daemon_mod, "_PLANTED_FIRE_MARKER_PATH", tmp_path / "fire") + monkeypatch.setattr(daemon_mod, "_PLANTED_FIRE_ROUND_COMPLETE_PATH", tmp_path / "rc") + server = _server(tmp_path) + server._auth_token = "test-token" + seen: dict = {} + + async def handle(req, ws=None, session=None, cancel_event=None, allow_tools=True): + # The wrapper, not `_run_tool_loop`: a disconnect cancels the turn, and the + # cancel lands before the loop body runs — so the assertion is about what + # the turn was **handed**, which is exactly what this test is about. + seen["tools"] = req.tools + + server._run_tool_loop_locked = handle + socket = _FakeSocket([ + {"type": "auth", "token": "test-token"}, + { + "type": "task", "id": "req-receive", "session_id": "tool-set-receive", + "cwd": str(tmp_path), "prompt": "go", "tools": ["read", "grep"], + }, + # The turn is started as a task, so the read loop has to be given one more + # frame to await before the spawned turn gets to run. + {"type": "list_history", "session_id": "tool-set-receive", "cwd": str(tmp_path)}, + ]) + + async def _drive(): + await server._handle_client(socket) + # The turn was started as a task; it needs the loop to keep turning after + # the connection has closed, or it is cancelled before its body runs. + await asyncio.sleep(0.2) + + asyncio.run(_drive()) + + assert seen.get("tools") == ["read", "grep"], ( + "the receiver dropped the field, so a producer can state a set and the turn " + "runs on the default one" + ) + + +def test_the_scheduler_states_the_scheduled_task_set(tmp_path, monkeypatch): + """The producer side, driven through the real cycle runner in test_scheduler.py.""" + # Kept as a pointer to the assertion's home: `_run_cycle_bounded` needs a + # websocket, a handler and a template, and test_scheduler.py already builds + # all three for the neighbouring field (`input_history`). + assert scheduled_task_tool_names() == default_tool_names(), ( + "today the two sets are equal; when they stop being equal, the assertion in " + "test_scheduler.py is what has to change with them" + )