diff --git a/src/lembas/services/agent/patch.py b/src/lembas/services/agent/patch.py
new file mode 100644
index 0000000..020a1c2
--- /dev/null
+++ b/src/lembas/services/agent/patch.py
@@ -0,0 +1,286 @@
+"""Applying a unified diff, and rendering one.
+
+`difflib` produces a unified diff and cannot apply one, so `render` uses it and
+`apply` is written here. No new dependency: hard rule 1 is about the browser,
+but a patch applier is fifty lines and pulling a package in for it would be
+worse than the fifty lines.
+
+Four behaviours carry the whole module, and each of them exists because of how
+models actually write patches rather than how the format is specified.
+
+**Fuzzy offset, exact content.** A hunk's `@@ -41,7 +41,8 @@` is a hint and
+nothing more. Models get line numbers wrong constantly -- they count from a
+truncated read, or from the file as it was three edits ago -- and get the
+context lines right. So the hinted position is tried first and then the file is
+scanned outward for an exact match of the context block. One match wins; more
+than one refuses, because guessing which of two identical blocks was meant is
+the one failure that silently corrupts a file.
+
+**Line endings are normalised in and restored out.** A CRLF file otherwise
+fails on every single hunk, on context that looks identical in the error
+message, which is unfixable from the model's side.
+
+**A blank context line may have lost its leading space.** Trailing whitespace
+is stripped by half the things a model's output passes through, so `""` is read
+as a blank context line rather than as a malformed one.
+
+**Nothing is written unless every hunk applies.** The new text is built whole in
+memory and handed back; a half-applied file is worse than a refused one, and the
+model cannot tell the difference without reading it again.
+"""
+
+from __future__ import annotations
+
+import difflib
+import re
+from dataclasses import dataclass
+
+# A patch bigger than this is a rewrite wearing a diff's clothes, and
+# `file_write` is the tool for that.
+MAX_HUNKS = 60
+
+# How far either side of the hinted line to look for the context block. Wide
+# enough for a file that has grown a few hundred lines since the model read it,
+# narrow enough that an accidental match is unlikely.
+MAX_DRIFT = 200
+
+_HEADER = re.compile(r"^@@\s*-(\d+)(?:,(\d+))?\s+\+(\d+)(?:,(\d+))?\s*@@")
+_NO_NEWLINE = "\\ No newline at end of file"
+
+
+class PatchError(Exception):
+ """A patch that did not apply, said precisely enough to retry from."""
+
+ def __init__(self, message: str, *, hunk: int = 0) -> None:
+ super().__init__(message)
+ self.message = message
+ self.hunk = hunk
+
+
+@dataclass(frozen=True)
+class Hunk:
+ old_start: int
+ old_count: int
+ new_start: int
+ new_count: int
+ # Each line still carrying its ' ', '+' or '-'.
+ lines: tuple[str, ...]
+ # A `\ No newline at end of file` marker followed a line this hunk *adds*,
+ # so the result is meant to end without one. Honoured only when the hunk
+ # actually reaches the end of the file -- git emits the marker for the old
+ # side too, and reading that as an instruction would strip a newline the
+ # patch never touched.
+ ends_without_newline: bool = False
+
+ @property
+ def before(self) -> tuple[str, ...]:
+ """The lines this hunk expects to find, without their markers."""
+ return tuple(line[1:] for line in self.lines if line[:1] in (" ", "-"))
+
+ @property
+ def after(self) -> tuple[str, ...]:
+ return tuple(line[1:] for line in self.lines if line[:1] in (" ", "+"))
+
+
+def parse(patch: str) -> list[Hunk]:
+ """Read a unified diff into hunks.
+
+ File headers are tolerated and ignored -- `diff --git`, `index`, `---`,
+ `+++` -- because models emit them by habit and refusing would cost a round
+ trip to say so. The `@@` header is required: without one there is nothing to
+ anchor against, and the resulting error is at least mechanical to fix.
+ """
+ hunks: list[Hunk] = []
+ state: dict = {"header": None, "body": [], "bare": False}
+
+ def flush() -> None:
+ if state["header"] is None:
+ return
+ hunks.append(
+ Hunk(
+ *state["header"],
+ lines=tuple(state["body"]),
+ ends_without_newline=state["bare"],
+ )
+ )
+ state["header"] = None
+ state["body"] = []
+ state["bare"] = False
+
+ body = (patch or "").replace("\r\n", "\n").replace("\r", "\n").split("\n")
+ # The patch's own final newline, not a blank context line. Without this every
+ # well-formed patch acquires one phantom line of context at the end and
+ # matches nothing -- which looks exactly like the model getting it wrong.
+ if body and body[-1] == "":
+ body.pop()
+
+ for raw in body:
+ matched = _HEADER.match(raw)
+ if matched:
+ flush()
+ state["header"] = (
+ int(matched.group(1)),
+ int(matched.group(2) or 1),
+ int(matched.group(3)),
+ int(matched.group(4) or 1),
+ )
+ continue
+
+ if state["header"] is None:
+ # Preamble. Anything before the first @@ is a file header we do not
+ # need: the path is a parameter, not something read out of the diff.
+ continue
+
+ if raw.startswith(_NO_NEWLINE):
+ # It describes whichever side the line above belonged to. Only the
+ # new side is an instruction; the old side is a description of the
+ # file we are about to read for ourselves.
+ if state["body"] and state["body"][-1][:1] in ("+", " "):
+ state["bare"] = True
+ continue
+ if raw[:1] in ("+", "-", " "):
+ state["body"].append(raw)
+ elif raw == "":
+ # A blank line that lost its leading space. Common enough to be the
+ # normal case rather than an exceptional one.
+ state["body"].append(" ")
+ else:
+ # A stray line inside a hunk -- a second `diff --git`, a signature.
+ # Ends the hunk rather than corrupting it.
+ flush()
+
+ flush()
+
+ if not hunks:
+ raise PatchError(
+ "That patch has no hunks. A patch needs at least one "
+ "`@@ -old,count +new,count @@` header, followed by the lines to "
+ "change: ' ' for context, '-' to remove, '+' to add."
+ )
+ if len(hunks) > MAX_HUNKS:
+ raise PatchError(
+ f"That patch has {len(hunks)} hunks, and {MAX_HUNKS} is the most "
+ f"that will be applied at once. Rewrite the file with file_write "
+ f"instead, or send the change in pieces."
+ )
+ return hunks
+
+
+def _find(lines: list[str], wanted: tuple[str, ...], hint: int, floor: int) -> int:
+ """Where `wanted` sits in `lines`, at or after `floor`. Raises if unclear."""
+ if not wanted:
+ # A pure insertion has no context to match. The hint is all there is.
+ return max(floor, min(hint, len(lines)))
+
+ span = len(wanted)
+ if hint >= floor and lines[hint : hint + span] == list(wanted):
+ return hint
+
+ matches = [
+ at
+ for at in range(max(floor, hint - MAX_DRIFT), min(len(lines) - span, hint + MAX_DRIFT) + 1)
+ if lines[at : at + span] == list(wanted)
+ ]
+ if len(matches) == 1:
+ return matches[0]
+ if len(matches) > 1:
+ raise PatchError(
+ f"Those context lines appear {len(matches)} times in the file, and "
+ f"the line numbers in the hunk header do not point at any of them, "
+ f"so there is no way to tell which was meant. Include more "
+ f"unchanged lines around the change."
+ )
+ raise PatchError("") # Filled in by the caller, which knows the hunk number.
+
+
+def apply(text: str, hunks: list[Hunk]) -> str:
+ """The file with every hunk applied, or a PatchError naming the first that
+ would not.
+
+ Hunks are applied in order against a cursor, so one cannot match inside
+ territory an earlier one already consumed -- which is what a duplicated or
+ overlapping hunk would otherwise do, applying the same change twice.
+ """
+ crlf = "\r\n" in text
+ lines = text.replace("\r\n", "\n").replace("\r", "\n").split("\n")
+ trailing = lines and lines[-1] == ""
+ if trailing:
+ lines.pop()
+
+ out: list[str] = []
+ cursor = 0
+ reached_end = False
+
+ for number, hunk in enumerate(hunks, start=1):
+ wanted = hunk.before
+ # A pure insertion names the line it goes *after*, not the line it
+ # replaces, so it is not off by one the way every other hunk is.
+ hint = hunk.old_start if hunk.old_count == 0 else max(hunk.old_start - 1, 0)
+ try:
+ at = _find(lines, wanted, hint, cursor)
+ except PatchError as exc:
+ raise _mismatch(number, hunk, lines, hint, exc.message) from None
+
+ out.extend(lines[cursor:at])
+ out.extend(hunk.after)
+ cursor = at + len(wanted)
+ reached_end = hunk.ends_without_newline and cursor >= len(lines)
+
+ out.extend(lines[cursor:])
+
+ result = "\n".join(out)
+ if trailing and not reached_end:
+ result += "\n"
+ return result.replace("\n", "\r\n") if crlf else result
+
+
+def _mismatch(number: int, hunk: Hunk, lines: list[str], hint: int, why: str) -> PatchError:
+ """The message the model retries from, so it has to say what is actually
+ there rather than only that something is wrong."""
+ if why:
+ return PatchError(
+ f"Hunk {number} did not apply. {why} Nothing was written.", hunk=number
+ )
+
+ expected = next((line[1:] for line in hunk.lines if line[:1] in (" ", "-")), "")
+ found = lines[hint] if 0 <= hint < len(lines) else "(past the end of the file)"
+ return PatchError(
+ f"Hunk {number} did not apply. It expects line {hint + 1} to be\n"
+ f" {expected}\n"
+ f"but the file has\n"
+ f" {found}\n"
+ f"and those lines are nowhere else nearby either. Nothing was written. "
+ f"Read the file again and send a patch that matches it.",
+ hunk=number,
+ )
+
+
+def render(before: str, after: str, path: str, *, max_lines: int = 200) -> str:
+ """A unified diff of one change, for the transcript.
+
+ Bounded here rather than at render time: this ends up in
+ `Message.tool_calls_json`, which is on the row forever and re-parsed on
+ every page load, and a generated file's diff can be larger than the file.
+ """
+ # splitlines, not split("\n"): a file's own final newline would otherwise be
+ # an empty last element, which difflib renders as a stray context line at
+ # the bottom of every diff -- and as a spurious change whenever one side has
+ # it and the other does not. The trailing-newline difference is invisible
+ # here as a result, which is right for a display and irrelevant to the write.
+ lines = list(
+ difflib.unified_diff(
+ before.replace("\r\n", "\n").splitlines(),
+ after.replace("\r\n", "\n").splitlines(),
+ fromfile=f"a/{path}",
+ tofile=f"b/{path}",
+ lineterm="",
+ n=3,
+ )
+ )
+ if len(lines) > max_lines:
+ dropped = len(lines) - max_lines
+ lines = lines[:max_lines] + [f"… ({dropped} more lines)"]
+ return "\n".join(lines)
+
+
+__all__ = ["MAX_DRIFT", "MAX_HUNKS", "Hunk", "PatchError", "apply", "parse", "render"]
diff --git a/src/lembas/services/agent/session.py b/src/lembas/services/agent/session.py
index be7fee4..14693f0 100644
--- a/src/lembas/services/agent/session.py
+++ b/src/lembas/services/agent/session.py
@@ -58,6 +58,26 @@ class AgentContext:
# this they would refuse the very thing that was approved -- the mode says
# "ask", and asking is exactly what happened.
approved: bool = False
+ # Absolute paths this reply has read. `file_edit` refuses a file that is not
+ # in here, because a patch written from memory against a file the model has
+ # not looked at is how a rewrite silently loses somebody's work.
+ #
+ # Here rather than on `Generation` for two reasons. Runners never see a
+ # Generation -- they get a `ToolContext`, which is a session-free snapshot
+ # precisely so nothing in a tool holds live state -- and a read path is a
+ # fact about the machine, which is what this class is.
+ #
+ # It is **shared with the approved copy**: `as_approved` is
+ # `dataclasses.replace`, which copies field references, so a path read
+ # through an approved call is visible here. That is wanted and is not
+ # obvious, so there is a test for it.
+ #
+ # It resets each reply, and that is correct rather than a limitation.
+ # `Message.tool_calls_json` is deliberately never replayed as context, so on
+ # the next turn the model does not have the file's contents either --
+ # requiring a re-read in the reply that edits is asking for something it
+ # needs anyway.
+ read_paths: set[str] = field(default_factory=set)
def executor(self) -> Executor:
return ssh_service.SshExecutor(self.spec, self.project_dir)
diff --git a/src/lembas/services/agent/tools.py b/src/lembas/services/agent/tools.py
index e4852dd..f187615 100644
--- a/src/lembas/services/agent/tools.py
+++ b/src/lembas/services/agent/tools.py
@@ -19,9 +19,10 @@ from __future__ import annotations
import json
import logging
+import posixpath
from typing import Any
-from lembas.services.agent import index, policy
+from lembas.services.agent import index, patch, policy
from lembas.services.agent.base import ExecError, ExecRequest
from lembas.services.agent.session import AgentContext
from lembas.services.tools import (
@@ -171,6 +172,37 @@ def _timeout(raw: Any, agent: AgentContext) -> float:
# --- Files ---------------------------------------------------------------------
+def _path_key(agent: AgentContext, path: str) -> str:
+ """One name for one file, so `./a.py` and `a.py` are the same file.
+
+ Relative paths are resolved against the project directory, which is what the
+ executor does with them, so the two cannot disagree about what was read.
+ """
+ if not posixpath.isabs(path) and agent.project_dir:
+ path = posixpath.join(agent.project_dir, path)
+ return posixpath.normpath(path)
+
+
+async def _current(agent: AgentContext, path: str) -> tuple[str, bool]:
+ """What is in the file now, and whether it is safe to diff against.
+
+ Best effort, and one extra SFTP round trip on every write -- see the note in
+ `_run_write`. A file that cannot be read and a file that does not exist are
+ the same thing over SFTP without a second trip for a stat, and both are
+ shown as a new file, which is what git does and is honest enough here.
+
+ Not diffable when the read came back at the ceiling: `read_file` truncates
+ and says so in the text rather than in a flag, so a file at `max_output` is
+ assumed truncated. Diffing a truncated original invents deletions of the
+ tail, which is worse than showing no diff at all.
+ """
+ try:
+ text = await agent.executor().read_file(path, max_bytes=agent.max_output)
+ except ExecError:
+ return "", True
+ return text, len(text) < agent.max_output
+
+
async def _run_read(context: ToolContext, args: dict[str, Any]) -> ToolOutcome:
agent = _agent(context)
path = str(args.get("path") or "").strip()
@@ -187,6 +219,10 @@ async def _run_read(context: ToolContext, args: dict[str, Any]) -> ToolOutcome:
exc.message, _event("file_read", agent, path, status="error", error=exc.message)
)
+ # What makes `file_edit` possible: a patch may only be applied to something
+ # this reply has actually looked at.
+ agent.read_paths.add(_path_key(agent, path))
+
return ToolOutcome(
text or "(the file is empty)",
_event("file_read", agent, path, status="ok", text=text[:MAX_EVENT_CHARS]),
@@ -206,6 +242,13 @@ async def _run_write(context: ToolContext, args: dict[str, Any]) -> ToolOutcome:
if not isinstance(content, str):
content = "" if content is None else json.dumps(content, ensure_ascii=False)
+ # One extra SFTP round trip per write, on the hottest agent operation, and a
+ # conscious trade. It buys the transcript a real diff instead of "1284
+ # bytes" -- which is the difference between being able to see what an agent
+ # did and having to go and look -- and it counts as having read the file, so
+ # a write followed by an edit works in one reply.
+ before, diffable = await _current(agent, path)
+
try:
written = await agent.executor().write_file(path, content)
except ExecError as exc:
@@ -213,6 +256,8 @@ async def _run_write(context: ToolContext, args: dict[str, Any]) -> ToolOutcome:
exc.message, _event("file_write", agent, path, status="error", error=exc.message)
)
+ agent.read_paths.add(_path_key(agent, path))
+
# The tree just changed, and this process is what changed it. The listing's
# TTL is for drift nobody can see coming; leaving five more minutes of a
# listing known to be wrong makes a model conclude the file it has just
@@ -220,10 +265,80 @@ async def _run_write(context: ToolContext, args: dict[str, Any]) -> ToolOutcome:
if agent.profile_id:
index.forget_dir(agent.profile_id, agent.project_dir)
- return ToolOutcome(
- f"Wrote {written} bytes to {path}.",
- _event("file_write", agent, path, status="ok", text=f"{written} bytes"),
- )
+ event = _event("file_write", agent, path, status="ok", text=f"{written} bytes")
+ if diffable and before != content:
+ event["diff"] = patch.render(before, content, path, max_lines=MAX_DIFF_LINES)
+
+ return ToolOutcome(f"Wrote {written} bytes to {path}.", event)
+
+
+async def _run_edit(context: ToolContext, args: dict[str, Any]) -> ToolOutcome:
+ """Change part of a file by applying a unified diff.
+
+ The read-first requirement is the whole point. A patch written from memory
+ against a file the model has not looked at either fails on context -- the
+ good case -- or matches something it did not mean, and `file_write`'s
+ failure mode is worse still: it silently drops everything the model did not
+ happen to recall. Making the read compulsory turns "lost half the file" into
+ "was told to read it first".
+ """
+ agent = _agent(context)
+ path = str(args.get("path") or "").strip()
+ if agent is None or not path:
+ return _no_connection_or_path("file_edit", agent, path)
+
+ if reason := _permitted(agent, "file_edit", RISK_WRITE):
+ return _refused("file_edit", agent, path, reason)
+
+ if _path_key(agent, path) not in agent.read_paths:
+ return ToolOutcome(
+ f"Read the file first! Nothing was written. Call file_read on {path} "
+ f"in this reply, then send a patch that matches what came back.",
+ _event("file_edit", agent, path, status="error", error="Not read yet."),
+ )
+
+ raw = args.get("patch")
+ if not isinstance(raw, str) or not raw.strip():
+ return ToolOutcome(
+ "No patch was given. Send a unified diff: one or more "
+ "`@@ -old,count +new,count @@` hunks.",
+ _event("file_edit", agent, path, status="error", error="No patch."),
+ )
+
+ before, diffable = await _current(agent, path)
+ try:
+ after = patch.apply(before, patch.parse(raw))
+ except patch.PatchError as exc:
+ # Returned, never raised: `run_tool`'s blanket catch would keep the
+ # model going but lose the detail, and the detail is what it retries
+ # from.
+ return ToolOutcome(
+ exc.message,
+ _event("file_edit", agent, path, status="error", error=exc.message[:200]),
+ )
+
+ if after == before:
+ return ToolOutcome(
+ f"That patch changes nothing in {path}. It is already as you want it.",
+ _event("file_edit", agent, path, status="ok", text="no change"),
+ )
+
+ try:
+ written = await agent.executor().write_file(path, after)
+ except ExecError as exc:
+ return ToolOutcome(
+ exc.message, _event("file_edit", agent, path, status="error", error=exc.message)
+ )
+
+ # Deliberately NOT index.forget_dir: an edit does not change the listing,
+ # because the file was already there. Forgetting it would cost the next
+ # reply either a wait on `INDEX_WAIT` or a turn with no listing at all, and
+ # buy nothing.
+ event = _event("file_edit", agent, path, status="ok", text=f"{written} bytes")
+ if diffable:
+ event["diff"] = patch.render(before, after, path, max_lines=MAX_DIFF_LINES)
+
+ return ToolOutcome(f"Updated {path} ({written} bytes).", event)
async def _run_list(context: ToolContext, args: dict[str, Any]) -> ToolOutcome:
@@ -363,9 +478,12 @@ def tool_defs(context: AgentContext | None = None) -> list[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."
+ "Create a text file, or replace an existing one entirely. A "
+ "relative path is taken from the project directory. Use this for a "
+ "new file, or when you are rewriting the whole thing. To change "
+ "part of a file that already exists, use file_edit instead: it is "
+ "cheaper, and it cannot silently lose the parts you did not mean "
+ "to touch."
),
parameters={
"type": "object",
@@ -378,6 +496,36 @@ def tool_defs(context: AgentContext | None = None) -> list[ToolDef]:
run=_run_write,
risk=RISK_WRITE,
),
+ ToolDef(
+ name="file_edit",
+ family=FAMILY_AGENT,
+ description=(
+ "Change part of a text file by applying a unified diff. You must "
+ "have read the file with file_read in this same reply first, or "
+ "this is refused — a patch written from memory is how a change "
+ "quietly becomes a rewrite.\n"
+ "\n"
+ "Send an ordinary patch: one or more `@@ -old,count +new,count @@` "
+ "hunks, each with about three unchanged lines of context on either "
+ "side of the change, ' ' for context, '-' to remove and '+' to add. "
+ "The line numbers may be approximate — the context lines must be "
+ "exact. Nothing is written unless every hunk applies, and you are "
+ "told which one failed and what the file has there instead."
+ ),
+ parameters={
+ "type": "object",
+ "properties": {
+ "path": {**_STRING, "description": "The file to change."},
+ "patch": {
+ **_STRING,
+ "description": "The unified diff to apply.",
+ },
+ },
+ "required": ["path", "patch"],
+ },
+ run=_run_edit,
+ risk=RISK_WRITE,
+ ),
ToolDef(
name="file_list",
family=FAMILY_AGENT,
@@ -426,4 +574,4 @@ def tool_defs(context: AgentContext | None = None) -> list[ToolDef]:
return defs
-__all__ = ["FAMILY_AGENT", "MAX_EVENT_CHARS", "tool_defs"]
+__all__ = ["FAMILY_AGENT", "MAX_DIFF_LINES", "MAX_EVENT_CHARS", "tool_defs"]
diff --git a/src/lembas/web/static/css/chat.css b/src/lembas/web/static/css/chat.css
index 717a061..6a3995d 100644
--- a/src/lembas/web/static/css/chat.css
+++ b/src/lembas/web/static/css/chat.css
@@ -400,6 +400,39 @@
overflow-y: auto;
}
+/* A diff, from a write or an update.
+
+ The same visual language as .tool-result__text above -- both answer "what did
+ it do", and two languages would suggest a difference that is not there. The
+ colours are --success and --danger rather than anything new: --success is
+ deliberately a different hue from --leaf so an added line does not read as the
+ brand accent, and --danger is already what an error border uses, so a removed
+ line reads as removed rather than as broken.
+
+ The padding is on the line, not on the block, so a highlighted row runs the
+ full width instead of stopping short of the rounded corner. */
+.diff {
+ margin: 0;
+ padding: var(--sp-2) 0;
+ border-radius: var(--radius-sm);
+ background: var(--bg-sunken);
+ font-family: var(--font-mono);
+ font-size: var(--text-xs);
+ line-height: var(--leading-relaxed);
+ max-height: 26em;
+ overflow: auto;
+}
+.diff__line {
+ display: block;
+ padding: 0 var(--sp-3);
+ white-space: pre-wrap;
+ overflow-wrap: anywhere;
+}
+.diff__line--add { background: var(--success-soft); color: var(--success); }
+.diff__line--del { background: var(--danger-soft); color: var(--danger); }
+.diff__line--meta { color: var(--ink-faint); }
+.diff__line--ctx { color: var(--ink-muted); }
+
/* --- The model asking you something --------------------------------------- */
/* Attributed to the model on purpose. A card styled like the application is a
card people answer with things they would not tell a chatbot. */
diff --git a/src/lembas/web/templates/chat/_tool_activity.html b/src/lembas/web/templates/chat/_tool_activity.html
index e972cc2..378420a 100644
--- a/src/lembas/web/templates/chat/_tool_activity.html
+++ b/src/lembas/web/templates/chat/_tool_activity.html
@@ -80,6 +80,41 @@
{{ event.text }}
{% endif %}
+ {% if event.diff %}
+ {#
+ What a write or an update changed, in the shape everybody already reads a
+ change in.
+
+ Every line is text off somebody's machine and is escaped exactly like the
+ rest of this file. The classification reads the first character and never
+ interprets the rest — a line is coloured, never parsed.
+
+ Header lines are checked BEFORE the bare +/- ones, or `+++ b/x.py` renders
+ as an addition and `--- a/x.py` as a removal at the top of every diff. The
+ test is against the `a/` and `b/` prefixes rather than against three
+ dashes, because a removed line whose own text begins with `--` produces
+ exactly three dashes too.
+
+ No whitespace between the spans: they are `display: block` inside a ,
+ so a newline between two of them is a blank line on screen.
+ #}
+
+ {%- for line in event.diff.split("\n") -%}
+ {%- if line.startswith('@@') or line.startswith('--- a/')
+ or line.startswith('+++ b/') or line.startswith('diff ') -%}
+ {%- set cls = 'meta' -%}
+ {%- elif line.startswith('+') -%}
+ {%- set cls = 'add' -%}
+ {%- elif line.startswith('-') -%}
+ {%- set cls = 'del' -%}
+ {%- else -%}
+ {%- set cls = 'ctx' -%}
+ {%- endif -%}
+ {{ line }}
+ {%- endfor -%}
+
+ {% endif %}
+
{% for result in event.results %}
{% set scheme = (result.url or "").split(":")[0] | lower %}
diff --git a/tests/test_agent_patch.py b/tests/test_agent_patch.py
new file mode 100644
index 0000000..90c93ee
--- /dev/null
+++ b/tests/test_agent_patch.py
@@ -0,0 +1,195 @@
+"""Applying a unified diff.
+
+Pure unit tests, no server and no database: this is where the behaviour that
+makes `file_edit` usable by a real model lives, and every case here is one that
+a real model produces.
+"""
+
+from __future__ import annotations
+
+import pytest
+
+from lembas.services.agent import patch
+
+
+def _apply(text: str, diff: str) -> str:
+ return patch.apply(text, patch.parse(diff))
+
+
+FILE = "\n".join(f"line {n}" for n in range(1, 21)) + "\n"
+
+
+# --- The ordinary case ----------------------------------------------------------
+def test_a_hunk_at_the_line_it_says_applies():
+ result = _apply(
+ FILE,
+ "@@ -4,3 +4,3 @@\n line 3\n-line 4\n+LINE FOUR\n line 5\n",
+ )
+ assert "LINE FOUR" in result
+ assert "line 4\n" not in result
+ assert result.count("\n") == FILE.count("\n"), "no lines gained or lost"
+
+
+def test_headers_are_tolerated():
+ """Models emit them by habit. Refusing costs a round trip to say so."""
+ result = _apply(
+ FILE,
+ "diff --git a/x.py b/x.py\nindex 1234567..89abcde 100644\n"
+ "--- a/x.py\n+++ b/x.py\n@@ -4,3 +4,3 @@\n line 3\n-line 4\n+LINE FOUR\n line 5\n",
+ )
+ assert "LINE FOUR" in result
+
+
+def test_several_hunks_apply_in_order():
+ result = _apply(
+ FILE,
+ "@@ -2,3 +2,3 @@\n line 1\n-line 2\n+TWO\n line 3\n"
+ "@@ -15,3 +15,3 @@\n line 14\n-line 15\n+FIFTEEN\n line 16\n",
+ )
+ assert "TWO" in result and "FIFTEEN" in result
+
+
+def test_a_pure_insertion_needs_no_context():
+ """And names the line it goes *after*, so it is not off by one the way
+ every other hunk is."""
+ result = _apply("a\nb\n", "@@ -1,0 +2,1 @@\n+inserted\n")
+ assert result == "a\ninserted\nb\n"
+
+
+# --- Line numbers drift, context does not ---------------------------------------
+def test_a_hunk_whose_line_numbers_are_wrong_still_applies():
+ """The single highest-value behaviour here. Models count from a truncated
+ read or from the file as it was three edits ago and get the numbers wrong;
+ they get the context right."""
+ result = _apply(
+ FILE,
+ "@@ -1,3 +1,3 @@\n line 11\n-line 12\n+TWELVE\n line 13\n",
+ )
+ assert "TWELVE" in result
+ assert "line 12\n" not in result
+
+
+def test_a_hunk_that_matches_nowhere_is_refused_and_names_what_is_there():
+ with pytest.raises(patch.PatchError) as caught:
+ _apply(FILE, "@@ -4,3 +4,3 @@\n nothing\n-like this\n+new\n at all\n")
+
+ message = caught.value.message
+ assert "Hunk 1 did not apply" in message
+ assert "Nothing was written" in message
+ assert "Read the file again" in message
+
+
+def test_ambiguous_context_is_refused_rather_than_guessed_at():
+ """The one failure that silently corrupts a file. Two identical blocks and a
+ hint pointing at neither: there is no way to tell which was meant."""
+ text = "start\nsame\nsame\nsame\nmiddle\nsame\nsame\nsame\nend\n"
+ with pytest.raises(patch.PatchError) as caught:
+ _apply(text, "@@ -50,3 +50,3 @@\n same\n-same\n+CHANGED\n same\n")
+
+ assert "appear" in caught.value.message
+ assert "more unchanged lines" in caught.value.message.lower()
+
+
+def test_drift_beyond_the_ceiling_is_not_searched():
+ long = "\n".join(f"line {n}" for n in range(1, 1000)) + "\n"
+ with pytest.raises(patch.PatchError):
+ _apply(long, "@@ -1,3 +1,3 @@\n line 900\n-line 901\n+NINE\n line 902\n")
+
+
+def test_nothing_is_written_when_a_later_hunk_fails():
+ """Atomic. A half-applied file is worse than a refused one, and the model
+ cannot tell the difference without reading it again."""
+ with pytest.raises(patch.PatchError) as caught:
+ _apply(
+ FILE,
+ "@@ -2,3 +2,3 @@\n line 1\n-line 2\n+TWO\n line 3\n"
+ "@@ -15,3 +15,3 @@\n bogus\n-nope\n+x\n also bogus\n",
+ )
+ assert caught.value.hunk == 2
+
+
+def test_hunks_out_of_order_are_refused():
+ """Otherwise a duplicated hunk applies the same change twice."""
+ with pytest.raises(patch.PatchError):
+ _apply(
+ FILE,
+ "@@ -15,3 +15,3 @@\n line 14\n-line 15\n+FIFTEEN\n line 16\n"
+ "@@ -2,3 +2,3 @@\n line 1\n-line 2\n+TWO\n line 3\n",
+ )
+
+
+# --- The things that break on real files ------------------------------------------
+def test_a_crlf_file_round_trips_as_crlf():
+ """Without normalising in and restoring out, every hunk on a Windows file
+ fails on context that looks identical in the error message."""
+ text = "alpha\r\nbeta\r\ngamma\r\n"
+ result = _apply(text, "@@ -1,3 +1,3 @@\n alpha\n-beta\n+BETA\n gamma\n")
+
+ assert result == "alpha\r\nBETA\r\ngamma\r\n"
+ assert "\n\n" not in result.replace("\r\n", "\n\n").replace("\n\n", "\r\n")
+
+
+def test_a_blank_context_line_with_no_leading_space_applies():
+ """Trailing whitespace is stripped by half the things a model's output
+ passes through, so this is the normal case rather than a malformed one."""
+ text = "alpha\n\ngamma\n"
+ result = _apply(text, "@@ -1,3 +1,3 @@\n alpha\n\n-gamma\n+GAMMA\n")
+ assert result == "alpha\n\nGAMMA\n"
+
+
+def test_a_file_with_no_trailing_newline_keeps_none():
+ result = _apply("alpha\nbeta", "@@ -1,2 +1,2 @@\n alpha\n-beta\n+BETA\n")
+ assert result == "alpha\nBETA"
+
+
+def test_the_no_newline_marker_on_the_new_side_removes_the_trailing_newline():
+ result = _apply(
+ "alpha\nbeta\n",
+ "@@ -1,2 +1,2 @@\n alpha\n-beta\n+BETA\n\\ No newline at end of file\n",
+ )
+ assert result == "alpha\nBETA"
+
+
+def test_the_no_newline_marker_on_the_old_side_is_not_an_instruction():
+ """git emits it for the old side too. Reading that as an instruction would
+ strip a newline the patch never touched."""
+ result = _apply(
+ "alpha\nbeta\n",
+ "@@ -1,2 +1,2 @@\n alpha\n-beta\n\\ No newline at end of file\n+BETA\n",
+ )
+ assert result == "alpha\nBETA\n"
+
+
+# --- Refusing the unusable ---------------------------------------------------------
+def test_a_patch_with_no_hunks_says_what_one_looks_like():
+ with pytest.raises(patch.PatchError) as caught:
+ patch.parse("just change line four please")
+ assert "@@" in caught.value.message
+
+
+def test_too_many_hunks_is_refused_and_points_at_file_write():
+ diff = "".join(
+ f"@@ -{n},1 +{n},1 @@\n-line {n}\n+LINE {n}\n" for n in range(1, patch.MAX_HUNKS + 5)
+ )
+ with pytest.raises(patch.PatchError) as caught:
+ patch.parse(diff)
+ assert "file_write" in caught.value.message
+
+
+# --- Rendering -----------------------------------------------------------------------
+def test_render_produces_a_diff_of_the_change():
+ diff = patch.render("alpha\nbeta\n", "alpha\nBETA\n", "x.py")
+ assert "-beta" in diff
+ assert "+BETA" in diff
+ assert "a/x.py" in diff
+
+
+def test_render_is_bounded():
+ """It goes on the message row forever and is re-parsed on every page load,
+ and a generated file's diff can be larger than the file."""
+ before = "\n".join(str(n) for n in range(500))
+ after = "\n".join(f"x{n}" for n in range(500))
+ diff = patch.render(before, after, "big.txt", max_lines=20)
+
+ assert len(diff.split("\n")) <= 21
+ assert "more lines" in diff
diff --git a/tests/test_agent_tools.py b/tests/test_agent_tools.py
index 29ae54d..76f8592 100644
--- a/tests/test_agent_tools.py
+++ b/tests/test_agent_tools.py
@@ -206,6 +206,193 @@ async def test_files_are_written_and_read_back(db, user_id, machine, tmp_path):
assert "note.txt" in listed.content
+# --- Changing part of a file -------------------------------------------------------
+def _context(db, user_id, machine, **kwargs):
+ chat, _profile = _setup(db, user_id, machine, mode=policy.MODE_AUTO, **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)
+
+
+async def test_editing_a_file_that_was_not_read_is_refused(db, user_id, machine, tmp_path):
+ """Both halves matter. The wording is what the model acts on; that nothing
+ was written is the actual guarantee."""
+ (tmp_path / "project" / "note.txt").write_text("alpha\nbeta\n")
+ context = _context(db, user_id, machine)
+
+ outcome = await tools_service.run_tool(
+ context,
+ "file_edit",
+ _json.dumps({"path": "note.txt", "patch": "@@ -1,2 +1,2 @@\n alpha\n-beta\n+BETA\n"}),
+ )
+
+ assert outcome.content.startswith("Read the file first!")
+ assert outcome.event["status"] == "error"
+ assert (tmp_path / "project" / "note.txt").read_text() == "alpha\nbeta\n"
+
+
+async def test_reading_then_editing_writes_the_new_text(db, user_id, machine, tmp_path):
+ (tmp_path / "project" / "note.txt").write_text("alpha\nbeta\ngamma\n")
+ context = _context(db, user_id, machine)
+
+ await tools_service.run_tool(context, "file_read", '{"path": "note.txt"}')
+ outcome = await tools_service.run_tool(
+ context,
+ "file_edit",
+ _json.dumps(
+ {"path": "note.txt", "patch": "@@ -1,3 +1,3 @@\n alpha\n-beta\n+BETA\n gamma\n"}
+ ),
+ )
+
+ assert outcome.event["status"] == "ok"
+ assert (tmp_path / "project" / "note.txt").read_text() == "alpha\nBETA\ngamma\n"
+
+
+async def test_a_relative_and_an_absolute_path_are_the_same_file(db, user_id, machine, tmp_path):
+ """`./note.txt` read and `note.txt` edited has to count as having read it,
+ or the check refuses the very thing it was meant to permit."""
+ target = tmp_path / "project" / "note.txt"
+ target.write_text("alpha\nbeta\n")
+ context = _context(db, user_id, machine)
+
+ await tools_service.run_tool(context, "file_read", '{"path": "./note.txt"}')
+ outcome = await tools_service.run_tool(
+ context,
+ "file_edit",
+ _json.dumps({"path": str(target), "patch": "@@ -1,2 +1,2 @@\n alpha\n-beta\n+BETA\n"}),
+ )
+
+ assert outcome.event["status"] == "ok", outcome.content
+
+
+async def test_a_failed_hunk_names_it_and_writes_nothing(db, user_id, machine, tmp_path):
+ (tmp_path / "project" / "note.txt").write_text("alpha\nbeta\n")
+ context = _context(db, user_id, machine)
+
+ await tools_service.run_tool(context, "file_read", '{"path": "note.txt"}')
+ outcome = await tools_service.run_tool(
+ context,
+ "file_edit",
+ _json.dumps({"path": "note.txt", "patch": "@@ -1,2 +1,2 @@\n nope\n-wrong\n+x\n"}),
+ )
+
+ assert outcome.event["status"] == "error"
+ assert "Hunk 1" in outcome.content
+ assert (tmp_path / "project" / "note.txt").read_text() == "alpha\nbeta\n"
+
+
+async def test_a_write_counts_as_having_read_it(db, user_id, machine, tmp_path):
+ """`_run_write` reads the old content for its diff anyway, so write-then-edit
+ works in one reply without a second round trip."""
+ context = _context(db, user_id, machine)
+
+ await tools_service.run_tool(
+ context, "file_write", '{"path": "new.txt", "content": "one\\ntwo\\n"}'
+ )
+ outcome = await tools_service.run_tool(
+ context,
+ "file_edit",
+ _json.dumps({"path": "new.txt", "patch": "@@ -1,2 +1,2 @@\n one\n-two\n+TWO\n"}),
+ )
+
+ assert outcome.event["status"] == "ok", outcome.content
+ assert (tmp_path / "project" / "new.txt").read_text() == "one\nTWO\n"
+
+
+async def test_the_read_set_survives_an_approval(db, user_id, machine, tmp_path):
+ """`as_approved` is `dataclasses.replace`, which copies field *references*,
+ so the set is shared with the per-call copy a runner actually gets. That is
+ wanted, and it is not obvious enough to leave unpinned."""
+ from dataclasses import replace
+
+ (tmp_path / "project" / "note.txt").write_text("alpha\nbeta\n")
+ context = _context(db, user_id, machine)
+ approved = replace(context, agent=context.agent.as_approved())
+
+ await tools_service.run_tool(approved, "file_read", '{"path": "note.txt"}')
+
+ assert context.agent.read_paths, "the read done under approval is not visible"
+
+
+async def test_an_edit_that_changes_nothing_says_so(db, user_id, machine, tmp_path):
+ (tmp_path / "project" / "note.txt").write_text("alpha\nbeta\n")
+ context = _context(db, user_id, machine)
+
+ await tools_service.run_tool(context, "file_read", '{"path": "note.txt"}')
+ outcome = await tools_service.run_tool(
+ context,
+ "file_edit",
+ _json.dumps({"path": "note.txt", "patch": "@@ -1,2 +1,2 @@\n alpha\n-beta\n+beta\n"}),
+ )
+
+ assert outcome.event["status"] == "ok"
+ assert "changes nothing" in outcome.content
+
+
+# --- The diff on the event ------------------------------------------------------------
+async def test_an_edit_carries_a_diff(db, user_id, machine, tmp_path):
+ (tmp_path / "project" / "note.txt").write_text("alpha\nbeta\ngamma\n")
+ context = _context(db, user_id, machine)
+
+ await tools_service.run_tool(context, "file_read", '{"path": "note.txt"}')
+ outcome = await tools_service.run_tool(
+ context,
+ "file_edit",
+ _json.dumps(
+ {"path": "note.txt", "patch": "@@ -1,3 +1,3 @@\n alpha\n-beta\n+BETA\n gamma\n"}
+ ),
+ )
+
+ diff = outcome.event["diff"]
+ assert "-beta" in diff
+ assert "+BETA" in diff
+
+
+async def test_writing_a_new_file_shows_it_as_all_additions(db, user_id, machine):
+ """Which is what git does, and the right display."""
+ context = _context(db, user_id, machine)
+
+ outcome = await tools_service.run_tool(
+ context, "file_write", '{"path": "fresh.txt", "content": "one\\ntwo\\n"}'
+ )
+
+ body = [
+ line
+ for line in outcome.event["diff"].split("\n")
+ if line and not line.startswith(("@@", "+++", "---"))
+ ]
+ assert body and all(line.startswith("+") for line in body), body
+
+
+async def test_overwriting_a_file_shows_what_changed(db, user_id, machine, tmp_path):
+ (tmp_path / "project" / "note.txt").write_text("alpha\nbeta\n")
+ context = _context(db, user_id, machine)
+
+ outcome = await tools_service.run_tool(
+ context, "file_write", '{"path": "note.txt", "content": "alpha\\nBETA\\n"}'
+ )
+
+ assert "-beta" in outcome.event["diff"]
+ assert "+BETA" in outcome.event["diff"]
+
+
+async def test_a_file_too_big_to_read_is_written_without_a_diff(db, user_id, machine, tmp_path):
+ """A truncated original would invent deletions of the tail, which is worse
+ than showing no diff at all."""
+ from lembas.services import settings_store as store
+
+ store.update(db, {"max_output_bytes": 1024}, key=store.AGENTS)
+ (tmp_path / "project" / "big.txt").write_text("x" * 4000)
+ context = _context(db, user_id, machine)
+
+ outcome = await tools_service.run_tool(
+ context, "file_write", '{"path": "big.txt", "content": "small"}'
+ )
+
+ assert outcome.event["status"] == "ok"
+ assert "diff" not in outcome.event
+
+
async def test_writing_a_file_drops_the_project_listing(db, user_id, machine):
"""Otherwise the model is shown a five-minute-old tree that it knows is
wrong, and concludes the file it has just created does not exist.
diff --git a/tests/test_tool_activity.py b/tests/test_tool_activity.py
index 5d3eb41..8e3a301 100644
--- a/tests/test_tool_activity.py
+++ b/tests/test_tool_activity.py
@@ -192,3 +192,60 @@ 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' in html
+ assert 'diff__line--add">+BETA' in html
+ assert 'diff__line--ctx"> alpha' in html
+ assert 'diff__line--meta">@@ -1,2 +1,2 @@' 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' in html
+ assert 'diff__line--meta">+++ b/x.py' 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' 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+",
+ }
+ )
+ assert "