mirror of
https://github.com/NCBM/plyngent.git
synced 2026-11-07 23:47:54 +08:00
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.
This commit is contained in:
@@ -569,7 +569,8 @@ class ChatAgent:
|
|||||||
- Tools default **off**. ``tools=True`` clones this agent's registry with
|
- Tools default **off**. ``tools=True`` clones this agent's registry with
|
||||||
a **fresh session** bag (unless *session_state* is passed) and the
|
a **fresh session** bag (unless *session_state* is passed) and the
|
||||||
given *instance_state* (CLI typically shares the host instance for
|
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()
|
text = user_text.strip()
|
||||||
if not text:
|
if not text:
|
||||||
|
|||||||
+45
-17
@@ -24,6 +24,7 @@ from plyngent.runtime import ProviderNotSupportedError
|
|||||||
if TYPE_CHECKING:
|
if TYPE_CHECKING:
|
||||||
from collections.abc import Awaitable, Sequence
|
from collections.abc import Awaitable, Sequence
|
||||||
|
|
||||||
|
from plyngent.agent import ToolRegistry
|
||||||
from plyngent.cli.state import ReplState, YoloMode
|
from plyngent.cli.state import ReplState, YoloMode
|
||||||
from plyngent.lmproto.openai_compatible.model import AnyChatMessage
|
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"
|
"works after resume, not only via readline history). Auto-retry: 10s/20s/30s.\n"
|
||||||
"\n"
|
"\n"
|
||||||
"Side questions: /btw … answers without changing the main session\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"
|
"Use --session ID or /resume to continue a prior chat after restart.\n"
|
||||||
"\n"
|
"\n"
|
||||||
'Multiline: start a message with """ then end a later line with """.\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")
|
@slash.command("btw")
|
||||||
@click.argument("question", nargs=-1, required=False)
|
@click.argument("question", nargs=-1, required=False)
|
||||||
@click.option(
|
@click.option(
|
||||||
"--tools/--no-tools",
|
"--tools",
|
||||||
"with_tools",
|
"tools_mode",
|
||||||
default=False,
|
type=click.Choice(["no", "read", "full"]),
|
||||||
|
default="read",
|
||||||
show_default=True,
|
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(
|
@click.option(
|
||||||
"--fresh",
|
"--fresh",
|
||||||
@@ -945,39 +948,60 @@ def btw_cmd(
|
|||||||
state: ReplState,
|
state: ReplState,
|
||||||
question: tuple[str, ...],
|
question: tuple[str, ...],
|
||||||
*,
|
*,
|
||||||
with_tools: bool,
|
tools_mode: str,
|
||||||
fresh: bool,
|
fresh: bool,
|
||||||
) -> None:
|
) -> None:
|
||||||
"""Ask a side question without changing the main session transcript or DB.
|
"""Ask a side question without changing the main session transcript or DB.
|
||||||
|
|
||||||
Uses the current model and (unless ``--fresh``) a copy of main history.
|
Uses the current model and (unless ``--fresh``) a copy of main history.
|
||||||
Tools are **off** by default; ``--tools`` clones the main tool registry with
|
Tools default to **read-only** (``--tools=read``): file/vcs/ask/fetch-GET
|
||||||
a fresh session bag and the shared instance workspace.
|
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()
|
text = " ".join(question).strip()
|
||||||
if not text:
|
if not text:
|
||||||
click.echo("usage: /btw [--tools] [--fresh] <question>")
|
click.echo("usage: /btw [--tools read|no|full] [--fresh] <question>")
|
||||||
return
|
return
|
||||||
if state.agent.pending_retry_text is not None:
|
if state.agent.pending_retry_text is not None:
|
||||||
click.echo("error: finish or /retry the main turn before /btw")
|
click.echo("error: finish or /retry the main turn before /btw")
|
||||||
return
|
return
|
||||||
if with_tools and (not state.tools_enabled or state.agent.tools is None):
|
|
||||||
|
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)")
|
click.echo("error: --tools requires tools on (/tools on)")
|
||||||
return
|
return
|
||||||
|
tools_mode = "no"
|
||||||
|
|
||||||
from plyngent.cli.display import render_events
|
from plyngent.cli.display import render_events
|
||||||
from plyngent.cli.retry import run_cancellable
|
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.secho("btw: ", fg="magenta", nl=False)
|
||||||
click.echo(text)
|
click.echo(text)
|
||||||
tools_arg: bool = with_tools
|
if tools_mode == "no":
|
||||||
aside_session = SessionState() if with_tools else None
|
tools_arg: ToolRegistry | bool = False
|
||||||
aside_instance = state.instance_state if with_tools else None
|
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:
|
async def _run() -> None:
|
||||||
await run_cancellable(
|
side = render_events(
|
||||||
render_events(
|
|
||||||
state.agent.run_aside(
|
state.agent.run_aside(
|
||||||
text,
|
text,
|
||||||
include_history=not fresh,
|
include_history=not fresh,
|
||||||
@@ -986,7 +1010,11 @@ def btw_cmd(
|
|||||||
session_state=aside_session,
|
session_state=aside_session,
|
||||||
)
|
)
|
||||||
)
|
)
|
||||||
)
|
if tools_mode == "read":
|
||||||
|
with read_only_context():
|
||||||
|
await run_cancellable(side)
|
||||||
|
else:
|
||||||
|
await run_cancellable(side)
|
||||||
|
|
||||||
try:
|
try:
|
||||||
_await(_run())
|
_await(_run())
|
||||||
|
|||||||
@@ -209,3 +209,91 @@ async def test_run_aside_tools_clones_registry_with_fresh_session() -> None:
|
|||||||
pass
|
pass
|
||||||
assert hits == ["ok"]
|
assert hits == ["ok"]
|
||||||
assert "aside" not in main_session.extras
|
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 == []
|
||||||
|
|||||||
@@ -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["reads"] == 4 # three entries + EOF
|
||||||
assert calls["echoes"] == 3 # the interrupting echo still counted, loop survived
|
assert calls["echoes"] == 3 # the interrupting echo still counted, loop survived
|
||||||
assert "interrupted" in capsys.readouterr().out
|
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
|
||||||
|
|||||||
Reference in New Issue
Block a user