Agent chats run commands, and stop to ask first
The four tools an agent chat has -- shell_run, file_read, file_write, file_list -- and the mode table wired into the loop that decides which of them stop for approval. Verified end to end against a real Kali container over SSH: the card shows the command, allowing it runs it there, and the file it writes is visible from outside. The mode is enforced in `_authorise`, in the generation loop, server-side, keyed on each tool's declared risk. Not in the prompt: a model is told which mode it is in so it behaves sensibly, but everything it reads -- a web page, a README, the output of the last command -- is untrusted, and a rule written only into a system message is one a poisoned file can argue with. Within an agent chat every call goes through the table, including the built-in ones, because notes_edit writes and Plan mode meaning "look but do not touch" has to mean that too. Two things this turned up. The runners re-check the mode as a backstop, and that backstop refused the very thing a person had just approved -- the mode says "ask", and asking was exactly what happened. Approval is now threaded per call, on a copy of the context, because a round runs its calls together and only some of them were allowed. And the harness said nothing at all, because `registry` maps an offered tool *name* back to a family and did not know the agent tools existed. So shell_run resolved to no family and the fragment naming the machine, the directory and the mode was never admitted. The same omission cost custom tools their guidance once already; there is a test for it now. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -41,6 +41,35 @@ MODE_HINTS = {
|
||||
MODE_PLAN: "Reads freely, changes nothing, and finishes by proposing a plan.",
|
||||
}
|
||||
|
||||
# What the *model* is told about the mode it is in. Different words from
|
||||
# MODE_HINTS, which describes it to a person: this is about how to behave, and
|
||||
# says the one thing that changes what a competent model does -- that being
|
||||
# stopped for approval is normal and worth batching for.
|
||||
MODE_GUIDANCE = {
|
||||
MODE_MANUAL: (
|
||||
"You are in **Manual** mode: everything you do is shown to them for "
|
||||
"approval first. Expect to be interrupted, and say what you are about "
|
||||
"to do before you do it."
|
||||
),
|
||||
MODE_EDIT: (
|
||||
"You are in **Edit** mode: you may read and write files freely, but "
|
||||
"every command is shown to them for approval first. Prefer reading and "
|
||||
"writing files over shelling out where both would work."
|
||||
),
|
||||
MODE_AUTO: (
|
||||
"You are in **Auto** mode: nothing is shown to them first. That is trust "
|
||||
"rather than permission — be as careful as you would be if each step "
|
||||
"were being watched, and stop to say so if you find yourself about to "
|
||||
"do something you could not undo."
|
||||
),
|
||||
MODE_PLAN: (
|
||||
"You are in **Plan** mode: read and explore freely, but change nothing. "
|
||||
"Anything that writes or runs will be stopped for approval, so do not "
|
||||
"rely on it. Finish by setting out what you would do, as steps, so it "
|
||||
"can be carried out afterwards."
|
||||
),
|
||||
}
|
||||
|
||||
ALLOW = "allow"
|
||||
ASK = "ask"
|
||||
|
||||
@@ -172,6 +201,7 @@ __all__ = [
|
||||
"MODES",
|
||||
"MODE_AUTO",
|
||||
"MODE_EDIT",
|
||||
"MODE_GUIDANCE",
|
||||
"MODE_HINTS",
|
||||
"MODE_LABELS",
|
||||
"MODE_MANUAL",
|
||||
|
||||
@@ -0,0 +1,130 @@
|
||||
"""What one agent chat is pointed at, resolved while a session is open.
|
||||
|
||||
Everything a runner needs travels in `AgentContext`: the machine, the decrypted
|
||||
credential, the mode in force, and the two lists that adjust it. Nothing is
|
||||
looked up later, for the reason `Endpoint` is a frozen copy of a `Connection`
|
||||
and `ToolContext` carries an owner id rather than a `User` -- a generation
|
||||
outlives the request that started it, and a detached instance is a trap.
|
||||
|
||||
The mode is read **once, at the start of the reply**, and deliberately does not
|
||||
change under a reply already in flight. Somebody switching to Auto halfway
|
||||
through must not retroactively approve what is already queued.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import logging
|
||||
from dataclasses import dataclass, field, replace
|
||||
from typing import Any
|
||||
|
||||
from sqlalchemy.orm import Session as DBSession
|
||||
|
||||
from lembas.db.models import KIND_AGENT, Chat, SshProfile, User
|
||||
from lembas.services import settings_store
|
||||
from lembas.services.agent import policy
|
||||
from lembas.services.agent import ssh as ssh_service
|
||||
from lembas.services.agent.base import Executor
|
||||
from lembas.services.agent.policy import Limits
|
||||
|
||||
log = logging.getLogger(__name__)
|
||||
|
||||
|
||||
@dataclass
|
||||
class AgentContext:
|
||||
"""The machine an agent chat acts on, and what it may do there."""
|
||||
|
||||
chat_id: str
|
||||
label: str
|
||||
project_dir: str
|
||||
mode: str = policy.MODE_MANUAL
|
||||
allow: tuple[str, ...] = ()
|
||||
deny: tuple[str, ...] = ()
|
||||
limits: Limits = field(default_factory=Limits)
|
||||
# Per-command bounds, from the instance settings.
|
||||
timeout: float = 60.0
|
||||
max_timeout: float = 600.0
|
||||
max_output: int = 64 * 1024
|
||||
# The decrypted credential. Held here and nowhere else, and cleared by
|
||||
# `generation` when the reply ends -- a finished Generation lingers five
|
||||
# minutes so late followers get the final frames, and a private key should
|
||||
# not linger with it.
|
||||
spec: dict[str, Any] = field(default_factory=dict)
|
||||
# Set only on the per-call copy handed to a runner whose call a person has
|
||||
# just allowed. The runners re-check the mode as a backstop, and without
|
||||
# this they would refuse the very thing that was approved -- the mode says
|
||||
# "ask", and asking is exactly what happened.
|
||||
approved: bool = False
|
||||
|
||||
def executor(self) -> Executor:
|
||||
return ssh_service.SshExecutor(self.spec, self.project_dir)
|
||||
|
||||
def clear(self) -> None:
|
||||
self.spec = {}
|
||||
|
||||
def as_approved(self) -> AgentContext:
|
||||
"""A copy of this context for one call a person has allowed."""
|
||||
return replace(self, approved=True)
|
||||
|
||||
|
||||
def profile_for(db: DBSession, chat: Chat, user: User | None) -> SshProfile | None:
|
||||
"""The connection this chat is pointed at, if it is still usable.
|
||||
|
||||
Ownership is re-checked here rather than trusted from when the chat was
|
||||
created: a profile can be deleted, disabled, or moved to a host whose key
|
||||
has not been confirmed since, and any of those should stop the chat acting
|
||||
rather than be discovered at the first command.
|
||||
"""
|
||||
if chat is None or chat.kind != KIND_AGENT or not chat.ssh_profile_id:
|
||||
return None
|
||||
|
||||
profile = db.get(SshProfile, chat.ssh_profile_id)
|
||||
if profile is None or not profile.enabled:
|
||||
return None
|
||||
if user is not None and profile.owner_id != user.id:
|
||||
return None
|
||||
return profile
|
||||
|
||||
|
||||
def resolve(db: DBSession, chat: Chat, user: User | None) -> AgentContext | None:
|
||||
"""This chat's agent setup, or None if it has none it can use.
|
||||
|
||||
None is the answer to every "no": not an agent chat, the feature switched
|
||||
off, the connection gone or disabled, SSH not installed. Each of those means
|
||||
the agent tools are not offered at all, which is better than offering a tool
|
||||
that fails on its first call.
|
||||
|
||||
A profile whose host key has never been confirmed is deliberately *not* one
|
||||
of them. The tools are offered and the failure is explicit, because "check
|
||||
the connection and accept its fingerprint" is a thing the reader can act on,
|
||||
while a silently missing tool is not.
|
||||
"""
|
||||
profile = profile_for(db, chat, user)
|
||||
if profile is None:
|
||||
return None
|
||||
|
||||
values = settings_store.agents(db)
|
||||
if not values.get("enabled"):
|
||||
return None
|
||||
if ssh_service.available():
|
||||
return None
|
||||
|
||||
return AgentContext(
|
||||
chat_id=chat.id,
|
||||
label=profile.label,
|
||||
project_dir=chat.project_dir or profile.default_dir or "",
|
||||
mode=chat.agent_mode if chat.agent_mode in policy.MODES else policy.MODE_MANUAL,
|
||||
allow=tuple(values.get("allow_default") or ()),
|
||||
deny=tuple(values.get("deny_default") or ()),
|
||||
limits=Limits(
|
||||
steps=int(values.get("max_steps") or 40),
|
||||
wall_seconds=float(values.get("max_wall_seconds") or 900),
|
||||
output_bytes=int(values.get("max_total_output_bytes") or 1024 * 1024),
|
||||
),
|
||||
timeout=float(values.get("default_timeout") or 60),
|
||||
max_timeout=float(values.get("max_timeout") or 600),
|
||||
max_output=int(values.get("max_output_bytes") or 64 * 1024),
|
||||
spec=ssh_service.spec_from(profile),
|
||||
)
|
||||
|
||||
|
||||
__all__ = ["AgentContext", "profile_for", "resolve"]
|
||||
@@ -0,0 +1,331 @@
|
||||
"""The four things an agent chat can do to the machine it is pointed at.
|
||||
|
||||
Two rules shape all of them.
|
||||
|
||||
**The descriptions say nothing about where.** A tool description is schema, sent
|
||||
verbatim and deliberately not editable, and it states facts about what a runner
|
||||
does. Which machine, which directory and which mode is in force are facts about
|
||||
*this chat*, so they live in the harness fragment where they can change without
|
||||
the schema changing under a model mid-conversation.
|
||||
|
||||
**Every runner re-checks the mode.** `_authorise` in the generation loop is the
|
||||
real gate and runs before any of this, but a backstop here means a future path
|
||||
that reaches `run_tool` directly -- a retry, a test, an admin re-run button --
|
||||
cannot walk past it. That is the same instinct that closed the registry hole:
|
||||
the check belongs where the action is, not only where the action was decided.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import json
|
||||
import logging
|
||||
from typing import Any
|
||||
|
||||
from lembas.services.agent import policy
|
||||
from lembas.services.agent.base import ExecError, ExecRequest
|
||||
from lembas.services.agent.session import AgentContext
|
||||
from lembas.services.tools import (
|
||||
RISK_EXECUTE,
|
||||
RISK_READ,
|
||||
RISK_WRITE,
|
||||
ToolContext,
|
||||
ToolDef,
|
||||
ToolOutcome,
|
||||
)
|
||||
|
||||
log = logging.getLogger(__name__)
|
||||
|
||||
FAMILY_AGENT = "agent"
|
||||
|
||||
# How much of a command's output is kept on the message row for the transcript,
|
||||
# separately from what the model reads. `max_output` is spent once; this is
|
||||
# stored on every message forever.
|
||||
MAX_EVENT_CHARS = 4000
|
||||
|
||||
_STRING = {"type": "string"}
|
||||
|
||||
|
||||
def _event(name: str, context: AgentContext, summary: str, **extra: Any) -> dict[str, Any]:
|
||||
return {
|
||||
"name": name,
|
||||
"kind": "agent",
|
||||
"label": f"{context.label}",
|
||||
"query": summary,
|
||||
"detail": context.project_dir or "",
|
||||
"results": [],
|
||||
**extra,
|
||||
}
|
||||
|
||||
|
||||
def _refused(name: str, context: AgentContext, summary: str, reason: str) -> ToolOutcome:
|
||||
return ToolOutcome(
|
||||
f"That was not allowed: {reason}",
|
||||
_event(name, context, summary, status="error", error=reason),
|
||||
)
|
||||
|
||||
|
||||
def _permitted(context: AgentContext, name: str, risk: str, command: str = "") -> str:
|
||||
"""Empty when this call may proceed, else why not.
|
||||
|
||||
The backstop. What it catches is a call arriving by a path that skipped
|
||||
`_authorise` -- a retry, a test, some future re-run button.
|
||||
|
||||
A call a person has just allowed carries `approved` and goes straight
|
||||
through. Without that this would refuse the very thing that was approved:
|
||||
the mode says "ask", and asking is precisely what happened.
|
||||
"""
|
||||
if context.approved:
|
||||
return ""
|
||||
|
||||
decision = policy.decide(
|
||||
mode=context.mode,
|
||||
risk=risk,
|
||||
tool_name=name,
|
||||
command=command,
|
||||
allow=context.allow,
|
||||
deny=context.deny,
|
||||
)
|
||||
if decision.verdict == policy.ALLOW:
|
||||
return ""
|
||||
return decision.reason or "it needs to be approved first."
|
||||
|
||||
|
||||
def _agent(context: ToolContext) -> AgentContext | None:
|
||||
return getattr(context, "agent", None)
|
||||
|
||||
|
||||
# --- Running a command --------------------------------------------------------
|
||||
async def _run_shell(context: ToolContext, args: dict[str, Any]) -> ToolOutcome:
|
||||
agent = _agent(context)
|
||||
command = str(args.get("command") or "").strip()
|
||||
if agent is None:
|
||||
return ToolOutcome(
|
||||
"This conversation is not connected to a machine, so nothing can be run.",
|
||||
{"name": "shell_run", "status": "error", "error": "No connection.", "results": []},
|
||||
)
|
||||
if not command:
|
||||
return _refused("shell_run", agent, "", "no command was given.")
|
||||
|
||||
if reason := _permitted(agent, "shell_run", RISK_EXECUTE, command):
|
||||
return _refused("shell_run", agent, command, reason)
|
||||
|
||||
timeout = _timeout(args.get("timeout"), agent)
|
||||
try:
|
||||
result = await agent.executor().run(
|
||||
ExecRequest(
|
||||
command=command,
|
||||
cwd=str(args.get("cwd") or "").strip(),
|
||||
timeout=timeout,
|
||||
max_bytes=agent.max_output,
|
||||
)
|
||||
)
|
||||
except ExecError as exc:
|
||||
return ToolOutcome(
|
||||
exc.message, _event("shell_run", agent, command, status="error", error=exc.message)
|
||||
)
|
||||
|
||||
body = result.output.strip()
|
||||
if result.timed_out:
|
||||
head = f"The command was stopped after {timeout:g}s."
|
||||
elif result.exit_status == 0:
|
||||
head = "" if body else "It ran, and printed nothing."
|
||||
else:
|
||||
head = f"It exited {result.exit_status}."
|
||||
|
||||
content = f"{head}\n\n{body}".strip() if head else body
|
||||
return ToolOutcome(
|
||||
content or "It ran, and printed nothing.",
|
||||
_event(
|
||||
"shell_run",
|
||||
agent,
|
||||
command,
|
||||
status="ok" if result.ok else "error",
|
||||
error="" if result.ok else head,
|
||||
text=body[:MAX_EVENT_CHARS],
|
||||
),
|
||||
)
|
||||
|
||||
|
||||
def _timeout(raw: Any, agent: AgentContext) -> float:
|
||||
"""What the model asked for, bounded by what an administrator allowed."""
|
||||
try:
|
||||
wanted = float(raw) if raw is not None else agent.timeout
|
||||
except (TypeError, ValueError):
|
||||
wanted = agent.timeout
|
||||
return min(max(wanted, 1.0), agent.max_timeout)
|
||||
|
||||
|
||||
# --- Files ---------------------------------------------------------------------
|
||||
async def _run_read(context: ToolContext, args: dict[str, Any]) -> ToolOutcome:
|
||||
agent = _agent(context)
|
||||
path = str(args.get("path") or "").strip()
|
||||
if agent is None or not path:
|
||||
return _no_connection_or_path("file_read", agent, path)
|
||||
|
||||
if reason := _permitted(agent, "file_read", RISK_READ):
|
||||
return _refused("file_read", agent, path, reason)
|
||||
|
||||
try:
|
||||
text = await agent.executor().read_file(path, max_bytes=agent.max_output)
|
||||
except ExecError as exc:
|
||||
return ToolOutcome(
|
||||
exc.message, _event("file_read", agent, path, status="error", error=exc.message)
|
||||
)
|
||||
|
||||
return ToolOutcome(
|
||||
text or "(the file is empty)",
|
||||
_event("file_read", agent, path, status="ok", text=text[:MAX_EVENT_CHARS]),
|
||||
)
|
||||
|
||||
|
||||
async def _run_write(context: ToolContext, args: dict[str, Any]) -> ToolOutcome:
|
||||
agent = _agent(context)
|
||||
path = str(args.get("path") or "").strip()
|
||||
if agent is None or not path:
|
||||
return _no_connection_or_path("file_write", agent, path)
|
||||
|
||||
if reason := _permitted(agent, "file_write", RISK_WRITE):
|
||||
return _refused("file_write", agent, path, reason)
|
||||
|
||||
content = args.get("content")
|
||||
if not isinstance(content, str):
|
||||
content = "" if content is None else json.dumps(content, ensure_ascii=False)
|
||||
|
||||
try:
|
||||
written = await agent.executor().write_file(path, content)
|
||||
except ExecError as exc:
|
||||
return ToolOutcome(
|
||||
exc.message, _event("file_write", agent, path, status="error", error=exc.message)
|
||||
)
|
||||
|
||||
return ToolOutcome(
|
||||
f"Wrote {written} bytes to {path}.",
|
||||
_event("file_write", agent, path, status="ok", text=f"{written} bytes"),
|
||||
)
|
||||
|
||||
|
||||
async def _run_list(context: ToolContext, args: dict[str, Any]) -> ToolOutcome:
|
||||
agent = _agent(context)
|
||||
if agent is None:
|
||||
return _no_connection_or_path("file_list", agent, "")
|
||||
path = str(args.get("path") or "").strip()
|
||||
|
||||
if reason := _permitted(agent, "file_list", RISK_READ):
|
||||
return _refused("file_list", agent, path, reason)
|
||||
|
||||
try:
|
||||
names = await agent.executor().list_dir(path)
|
||||
except ExecError as exc:
|
||||
return ToolOutcome(
|
||||
exc.message, _event("file_list", agent, path, status="error", error=exc.message)
|
||||
)
|
||||
|
||||
where = path or agent.project_dir or "."
|
||||
body = "\n".join(names) if names else "(empty)"
|
||||
return ToolOutcome(
|
||||
f"{where}:\n{body}",
|
||||
_event("file_list", agent, where, status="ok", text=body[:MAX_EVENT_CHARS]),
|
||||
)
|
||||
|
||||
|
||||
def _no_connection_or_path(name: str, agent: AgentContext | None, path: str) -> ToolOutcome:
|
||||
if agent is None:
|
||||
return ToolOutcome(
|
||||
"This conversation is not connected to a machine.",
|
||||
{"name": name, "status": "error", "error": "No connection.", "results": []},
|
||||
)
|
||||
return _refused(name, agent, path, "no path was given.")
|
||||
|
||||
|
||||
# --- The definitions -----------------------------------------------------------
|
||||
def tool_defs(context: AgentContext | None = None) -> list[ToolDef]:
|
||||
"""The agent tools, bound to one chat's machine.
|
||||
|
||||
`None` yields the same definitions unbound, which is what `tools.registry`
|
||||
needs: it maps an offered tool *name* back to its family and has no chat to
|
||||
resolve. Their runners still work -- they report that the conversation is
|
||||
not connected to a machine, which is true.
|
||||
"""
|
||||
return [
|
||||
ToolDef(
|
||||
name="shell_run",
|
||||
family=FAMILY_AGENT,
|
||||
description=(
|
||||
"Run a shell command and read back everything it printed, stdout "
|
||||
"and stderr together. Each call is a fresh shell, so a `cd` in one "
|
||||
"does not carry into the next — pass `cwd` instead. Nothing can "
|
||||
"answer a prompt, so pass the flags that make a command "
|
||||
"non-interactive rather than waiting for it to ask."
|
||||
),
|
||||
parameters={
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"command": {**_STRING, "description": "The command line to run."},
|
||||
"cwd": {
|
||||
**_STRING,
|
||||
"description": "Where to run it. Defaults to the project directory.",
|
||||
},
|
||||
"timeout": {
|
||||
"type": "number",
|
||||
"description": "Seconds to allow. Bounded by the instance settings.",
|
||||
},
|
||||
},
|
||||
"required": ["command"],
|
||||
},
|
||||
run=_run_shell,
|
||||
risk=RISK_EXECUTE,
|
||||
),
|
||||
ToolDef(
|
||||
name="file_read",
|
||||
family=FAMILY_AGENT,
|
||||
description=(
|
||||
"Read a text file. A relative path is taken from the project "
|
||||
"directory. Large files are cut off at the end rather than "
|
||||
"refused, and you are told when that happened."
|
||||
),
|
||||
parameters={
|
||||
"type": "object",
|
||||
"properties": {"path": {**_STRING, "description": "The file to read."}},
|
||||
"required": ["path"],
|
||||
},
|
||||
run=_run_read,
|
||||
risk=RISK_READ,
|
||||
),
|
||||
ToolDef(
|
||||
name="file_write",
|
||||
family=FAMILY_AGENT,
|
||||
description=(
|
||||
"Write a text file, replacing it entirely if it already exists. "
|
||||
"A relative path is taken from the project directory. Read a file "
|
||||
"before rewriting it unless you are certain what is in it."
|
||||
),
|
||||
parameters={
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"path": {**_STRING, "description": "The file to write."},
|
||||
"content": {**_STRING, "description": "Its whole new contents."},
|
||||
},
|
||||
"required": ["path", "content"],
|
||||
},
|
||||
run=_run_write,
|
||||
risk=RISK_WRITE,
|
||||
),
|
||||
ToolDef(
|
||||
name="file_list",
|
||||
family=FAMILY_AGENT,
|
||||
description=(
|
||||
"List a directory. Defaults to the project directory. Use this "
|
||||
"before guessing at a path."
|
||||
),
|
||||
parameters={
|
||||
"type": "object",
|
||||
"properties": {"path": {**_STRING, "description": "The directory to list."}},
|
||||
"required": [],
|
||||
},
|
||||
run=_run_list,
|
||||
risk=RISK_READ,
|
||||
),
|
||||
]
|
||||
|
||||
|
||||
__all__ = ["FAMILY_AGENT", "MAX_EVENT_CHARS", "tool_defs"]
|
||||
Reference in New Issue
Block a user