From b16b2547b8a1b105c3195d400f76de5d8d3f7a79 Mon Sep 17 00:00:00 2001 From: worldmozara Date: Sat, 15 Aug 2026 16:52:02 +0800 Subject: [PATCH] cli: /btw --tools [no|read|full] with read-only default /btw now defaults to --tools=read: the side turn gets only READ_ONLY tools under a read-only context (fetch restricted to GET), with the main session view so read-only reads (todo_list) see real state. --tools=full keeps the previous whole-registry clone with a fresh session; --tools=no turns tools off. When tools are off, the default read silently falls back to no tools, while an explicit --tools read|full errors as before. --- src/plyngent/agent/chat.py | 3 +- src/plyngent/cli/slash.py | 78 +++++++++++++------- tests/test_agent/test_run_aside.py | 88 ++++++++++++++++++++++ tests/test_cli/test_repl_commands.py | 105 +++++++++++++++++++++++++++ 4 files changed, 248 insertions(+), 26 deletions(-) diff --git a/src/plyngent/agent/chat.py b/src/plyngent/agent/chat.py index 4ce97c9..6010cee 100644 --- a/src/plyngent/agent/chat.py +++ b/src/plyngent/agent/chat.py @@ -569,7 +569,8 @@ class ChatAgent: - Tools default **off**. ``tools=True`` clones this agent's registry with a **fresh session** bag (unless *session_state* is passed) and the given *instance_state* (CLI typically shares the host instance for - workspace identity). + workspace identity). A caller-built :class:`ToolRegistry` (e.g. a + ``clone(read_only_only=True)`` subset) is used as-is. """ text = user_text.strip() if not text: diff --git a/src/plyngent/cli/slash.py b/src/plyngent/cli/slash.py index d26eb20..1965600 100644 --- a/src/plyngent/cli/slash.py +++ b/src/plyngent/cli/slash.py @@ -24,6 +24,7 @@ from plyngent.runtime import ProviderNotSupportedError if TYPE_CHECKING: from collections.abc import Awaitable, Sequence + from plyngent.agent import ToolRegistry from plyngent.cli.state import ReplState, YoloMode from plyngent.lmproto.openai_compatible.model import AnyChatMessage @@ -83,7 +84,8 @@ HELP_FOOTER = ( "works after resume, not only via readline history). Auto-retry: 10s/20s/30s.\n" "\n" "Side questions: /btw … answers without changing the main session\n" - "(optional --tools / --fresh). Tab completes slash commands and arguments.\n" + "(--tools=read by default: read-only tools; --tools no/full opt-in; --fresh).\n" + "Tab completes slash commands and arguments.\n" "Use --session ID or /resume to continue a prior chat after restart.\n" "\n" 'Multiline: start a message with """ then end a later line with """.\n' @@ -928,11 +930,12 @@ def _format_tool_tags(tags: object) -> str: @slash.command("btw") @click.argument("question", nargs=-1, required=False) @click.option( - "--tools/--no-tools", - "with_tools", - default=False, + "--tools", + "tools_mode", + type=click.Choice(["no", "read", "full"]), + default="read", show_default=True, - help="Allow tools on the side turn (shared workspace; forked session grants/todo).", + help=("Tool access on the side turn: read = read-only tools only (default), no = no tools, full = all tools."), ) @click.option( "--fresh", @@ -945,48 +948,73 @@ def btw_cmd( state: ReplState, question: tuple[str, ...], *, - with_tools: bool, + tools_mode: str, fresh: bool, ) -> None: """Ask a side question without changing the main session transcript or DB. Uses the current model and (unless ``--fresh``) a copy of main history. - Tools are **off** by default; ``--tools`` clones the main tool registry with - a fresh session bag and the shared instance workspace. + Tools default to **read-only** (``--tools=read``): file/vcs/ask/fetch-GET + reads with no state mutation. ``--tools=no`` turns tools off and + ``--tools=full`` clones the whole main registry with a fresh session bag + and the shared instance workspace. """ text = " ".join(question).strip() if not text: - click.echo("usage: /btw [--tools] [--fresh] ") + click.echo("usage: /btw [--tools read|no|full] [--fresh] ") return if state.agent.pending_retry_text is not None: click.echo("error: finish or /retry the main turn before /btw") return - if with_tools and (not state.tools_enabled or state.agent.tools is None): - click.echo("error: --tools requires tools on (/tools on)") - return + + tools_on = state.tools_enabled and state.agent.tools is not None + explicit = click.get_current_context().get_parameter_source("tools_mode") is not click.core.ParameterSource.DEFAULT + if tools_mode != "no" and not tools_on: + # Explicitly requesting tools while they are off is an error; the + # default (read) silently falls back to no tools so bare /btw still + # works under /tools off. + if tools_mode == "full" or explicit: + click.echo("error: --tools requires tools on (/tools on)") + return + tools_mode = "no" from plyngent.cli.display import render_events from plyngent.cli.retry import run_cancellable - from plyngent.tools.context import SessionState + from plyngent.tools.context import SessionState, read_only_context click.secho("btw: ", fg="magenta", nl=False) click.echo(text) - tools_arg: bool = with_tools - aside_session = SessionState() if with_tools else None - aside_instance = state.instance_state if with_tools else None + if tools_mode == "no": + tools_arg: ToolRegistry | bool = False + aside_session = None + aside_instance = None + elif tools_mode == "full": + tools_arg = True + aside_session = SessionState() + aside_instance = state.instance_state + else: # read + assert state.agent.tools is not None + # Read-only clone: keep the main session view so read-only session + # reads (e.g. todo_list) see the real state (they cannot mutate it). + tools_arg = state.agent.tools.clone(read_only_only=True) + aside_session = state.session_state + aside_instance = state.instance_state async def _run() -> None: - await run_cancellable( - render_events( - state.agent.run_aside( - text, - include_history=not fresh, - tools=tools_arg, - instance_state=aside_instance, - session_state=aside_session, - ) + side = render_events( + state.agent.run_aside( + text, + include_history=not fresh, + tools=tools_arg, + instance_state=aside_instance, + session_state=aside_session, ) ) + if tools_mode == "read": + with read_only_context(): + await run_cancellable(side) + else: + await run_cancellable(side) try: _await(_run()) diff --git a/tests/test_agent/test_run_aside.py b/tests/test_agent/test_run_aside.py index b176fb9..a442613 100644 --- a/tests/test_agent/test_run_aside.py +++ b/tests/test_agent/test_run_aside.py @@ -209,3 +209,91 @@ async def test_run_aside_tools_clones_registry_with_fresh_session() -> None: pass assert hits == ["ok"] assert "aside" not in main_session.extras + + +async def test_run_aside_read_only_clone_excludes_write_tools() -> None: + """A read-only-cloned registry passed to run_aside exposes no write tools.""" + hits: list[str] = [] + + @tool(tags=ToolTag.LOCAL | ToolTag.READ_ONLY, register=False) + async def peek() -> str: + hits.append("peek") + return "peeked" + + @tool(tags=ToolTag.LOCAL, register=False) + async def write_thing() -> str: + hits.append("write") + return "wrote" + + registry = ToolRegistry([peek, write_thing], auto_bind_state=True) + + def make_client(first_tool: str | None) -> Any: + calls = 0 + + class _Client: + @overload + async def chat_completions( + self, param: ChatCompletionsParam, *, stream: Literal[False] = False + ) -> ChatCompletionResponse: ... + + @overload + async def chat_completions( + self, param: ChatCompletionsParam, *, stream: Literal[True] + ) -> AsyncIterator[ChatCompletionChunk]: ... + + async def chat_completions( + self, param: ChatCompletionsParam, *, stream: bool = False + ) -> ChatCompletionResponse | AsyncIterator[ChatCompletionChunk]: + del stream, param + nonlocal calls + calls += 1 + if calls == 1 and first_tool is not None: + from plyngent.lmproto.openai_compatible.model import ( + AssistantFunctionTool, + AssistantFunctionToolCall, + ) + + message = AssistantChatMessage( + content="", + tool_calls=[ + AssistantFunctionToolCall( + id="c1", + function=AssistantFunctionTool(name=first_tool, arguments="{}"), + ) + ], + ) + finish = "tool_calls" + else: + message = AssistantChatMessage(content="done") + finish = "stop" + return ChatCompletionResponse( + id="1", + object="chat.completion", + created=0, + model="m", + choices=[ + ChatCompletionChoice( + index=0, + message=message, + logprobs={}, + finish_reason=finish, + ) + ], + system_fingerprint="", + usage={}, + ) + + return _Client() + + # Read-only tool is available to the side turn. + agent_peek = ChatAgent(make_client("peek"), model="m", tools=registry, stream=False) + async for _ in agent_peek.run_aside("go", tools=registry.clone(read_only_only=True)): + pass + assert hits == ["peek"] + + # Write tool is not in the read-only clone: the call errors, never runs. + hits.clear() + agent_write = ChatAgent(make_client("write_thing"), model="m", tools=registry, stream=False) + async for _ in agent_write.run_aside("go", tools=registry.clone(read_only_only=True)): + pass + assert hits == [] diff --git a/tests/test_cli/test_repl_commands.py b/tests/test_cli/test_repl_commands.py index ad756b9..a603fcf 100644 --- a/tests/test_cli/test_repl_commands.py +++ b/tests/test_cli/test_repl_commands.py @@ -633,3 +633,108 @@ async def test_repl_second_ctrl_c_after_cancel_returns_to_prompt( assert calls["reads"] == 4 # three entries + EOF assert calls["echoes"] == 3 # the interrupting echo still counted, loop survived assert "interrupted" in capsys.readouterr().out + + +async def test_btw_default_read_falls_back_when_tools_off(state: ReplState, capsys: pytest.CaptureFixture[str]) -> None: + """Bare /btw (default --tools=read) still works under /tools off: no tools.""" + assert state.tools_enabled is False + state.agent.stream = False # DummyClient only completes via non-stream path + assert await handle_slash(state, "/btw hello") is True + out = capsys.readouterr().out + assert "btw: hello" in out + assert "btw done" in out + assert "error" not in out + + +async def test_btw_explicit_tools_errors_when_off(state: ReplState, capsys: pytest.CaptureFixture[str]) -> None: + """Explicitly requesting read/full tools while they are off is an error.""" + state.agent.stream = False # DummyClient only completes via non-stream path + assert await handle_slash(state, "/btw --tools read q") is True + assert "error: --tools requires tools on" in capsys.readouterr().out + assert await handle_slash(state, "/btw --tools full q") is True + assert "error: --tools requires tools on" in capsys.readouterr().out + # Explicit no is fine. + assert await handle_slash(state, "/btw --tools no q") is True + out = capsys.readouterr().out + assert "btw done" in out + assert "error" not in out + + +async def test_btw_read_mode_wires_read_only_clone( + state: ReplState, + monkeypatch: pytest.MonkeyPatch, + capsys: pytest.CaptureFixture[str], +) -> None: + """--tools=read passes a read-only clone + read-only context + main session.""" + + from plyngent.agent import ToolRegistry, ToolTag, tool + from plyngent.tools.context import is_read_only_context + + @tool(tags=ToolTag.LOCAL | ToolTag.READ_ONLY, register=False) + async def ro_tool() -> str: + return "ro" + + @tool(tags=ToolTag.LOCAL, register=False) + async def w_tool() -> str: + return "w" + + state.tools_enabled = True + state.agent.tools = ToolRegistry([ro_tool, w_tool], auto_bind_state=True) + captured: dict[str, object] = {} + + async def fake_run_aside(_text: str, **kwargs: object) -> AsyncIterator[object]: + captured["tools"] = kwargs.get("tools") + captured["session_state"] = kwargs.get("session_state") + captured["read_only"] = is_read_only_context() + if False: + yield None + + monkeypatch.setattr(state.agent, "run_aside", fake_run_aside) + + assert await handle_slash(state, "/btw --tools read hi") is True + assert "btw done" in capsys.readouterr().out + + tools = captured["tools"] + assert isinstance(tools, ToolRegistry) + assert {t.name for t in tools.definitions()} == {"ro_tool"} + assert captured["session_state"] is state.session_state + assert captured["read_only"] is True + + +async def test_btw_full_mode_clones_full_registry( + state: ReplState, + monkeypatch: pytest.MonkeyPatch, + capsys: pytest.CaptureFixture[str], +) -> None: + """--tools=full keeps the whole registry with a fresh session, no context.""" + + from plyngent.agent import ToolRegistry, ToolTag, tool + from plyngent.tools.context import is_read_only_context + + @tool(tags=ToolTag.LOCAL | ToolTag.READ_ONLY, register=False) + async def ro_tool() -> str: + return "ro" + + @tool(tags=ToolTag.LOCAL, register=False) + async def w_tool() -> str: + return "w" + + state.tools_enabled = True + state.agent.tools = ToolRegistry([ro_tool, w_tool], auto_bind_state=True) + captured: dict[str, object] = {} + + async def fake_run_aside(_text: str, **kwargs: object) -> AsyncIterator[object]: + captured["tools"] = kwargs.get("tools") + captured["session_state"] = kwargs.get("session_state") + captured["read_only"] = is_read_only_context() + if False: + yield None + + monkeypatch.setattr(state.agent, "run_aside", fake_run_aside) + + assert await handle_slash(state, "/btw --tools full hi") is True + assert "btw done" in capsys.readouterr().out + + assert captured["tools"] is True + assert captured["session_state"] is not state.session_state + assert captured["read_only"] is False