feat: Add edit_file tool + file-change diffs (#1239)
* Add edit_file tool + file-change diffs
edit_file is an exact old_string -> new_string replacement on a file on disk
(fails if old_string is missing or non-unique unless replace_all); write_file
also returns a unified diff. Diffs render collapsed in the tool bubble
(filename + +adds/-dels, theme colors); the raw JSON command box is hidden.
Security: edit_file is a sensitive filesystem-write tool, treated everywhere
write_file is —
- added to NON_ADMIN_BLOCKED_TOOLS (is_public_blocked_tool / blocked_tools_for_owner),
so on auth-enabled deployments a non-admin cannot run it; execute_tool_block
refuses it for non-admin owners.
- confined by the same path policy as read_file/write_file (allowlist +
sensitive-file deny) via _resolve_tool_path.
Disambiguation in tool descriptions + bash prompt: edit_file/write_file are the
only way to write files (they show a diff) — never edit_document (editor panel)
or a bash heredoc/redirect.
Tests (tests/test_edit_file.py): non-admin block (policy + execution gate),
successful edit, not-found old_string, non-unique old_string (+ replace_all),
and path outside the allowed roots.
Files: src/tool_execution.py, src/agent_loop.py, src/tool_schemas.py,
src/agent_tools.py, src/tool_index.py, static/js/chat.js, static/style.css,
tests/test_edit_file.py.
* Drop redundant import os in write_file closure
os is already imported at module top.
This commit is contained in:
Kenny Van de Maele
committed by
GitHub
GitHub
parent
147d1fbde6
commit
7443c36bd9
@@ -177,6 +177,7 @@ TOOL_SECTIONS = {
|
||||
<shell command>
|
||||
```
|
||||
Run any shell command. Output is returned to you. Use for: installing packages, checking files, git, curl, system info, etc.
|
||||
NEVER use bash to create or change files — no `>`/`>>` redirects, no heredocs (`cat > f << 'EOF'`), no `tee`, `sed -i`, `awk -i`, no `python -c` that writes. To CREATE or fully rewrite a file use `write_file`; to change part of an existing file use `edit_file`. Those show a diff and are the ONLY allowed way to write files. (bash is for read-only inspection: `ls`, `cat` to READ, `grep`, `git status`/`git diff`, builds, installs.)
|
||||
For LONG-running commands (package installs, pip/npm, ffmpeg, model downloads, training, builds — anything that may take more than ~20s), make the FIRST line `#!bg` to run it in the BACKGROUND. You get a job id back immediately and are automatically re-invoked with the full output when it finishes — so you never block the chat waiting. Example:
|
||||
```bash
|
||||
#!bg
|
||||
@@ -220,6 +221,12 @@ Read a file and return its contents.""",
|
||||
```
|
||||
Write content to a file. First line is the path, rest is the content.""",
|
||||
|
||||
"edit_file": """\
|
||||
```edit_file
|
||||
{"path": "<file path>", "old_string": "<exact text to replace>", "new_string": "<replacement>", "replace_all": false}
|
||||
```
|
||||
Edit an EXISTING file by exact string replacement. PREFER this over bash (sed/echo/redirects) for changing files — it shows a before/after diff. `old_string` must match the file exactly and be unique unless `replace_all` is true. Use write_file to create a new file.""",
|
||||
|
||||
"create_document": """\
|
||||
```create_document
|
||||
<title>
|
||||
@@ -236,7 +243,7 @@ old text to find
|
||||
new replacement text
|
||||
<<<END>>>
|
||||
```
|
||||
PREFERRED way to change an existing document. Find exact text and replace it. Multiple FIND/REPLACE blocks per call OK. Use this for any edit smaller than a full rewrite — adding a function, fixing a bug, tweaking a section, renaming things. **If a document is open in the editor, treat it as the user's current context: don't ask which file they mean, and don't create a new one — just edit_document the active one.** Do NOT re-send the whole file with update_document for small changes.""",
|
||||
Edit a document OPEN IN THE EDITOR PANEL — NOT a file on disk. For files on disk (home folder, project files, any real path like ~/sweden.txt) use `edit_file` instead. Find exact text and replace it. Multiple FIND/REPLACE blocks per call OK. Use for any edit smaller than a full rewrite. **If a document is open in the editor, treat it as the user's current context: don't ask which file they mean, and don't create a new one — just edit_document the active one.** Do NOT re-send the whole file with update_document for small changes.""",
|
||||
|
||||
"update_document": """\
|
||||
```update_document
|
||||
@@ -2219,6 +2226,9 @@ async def stream_agent_loop(
|
||||
if result.get("images"):
|
||||
img = result["images"][0]
|
||||
tool_output_data["screenshot"] = f"data:{img['mimeType']};base64,{img['data']}"
|
||||
# Forward a file-write diff for inline before/after rendering
|
||||
if "diff" in result:
|
||||
tool_output_data["diff"] = result["diff"]
|
||||
yield f'data: {json.dumps(tool_output_data)}\n\n'
|
||||
|
||||
# Native document tools open in the editor + carry the REAL doc id.
|
||||
@@ -2261,6 +2271,10 @@ async def stream_agent_loop(
|
||||
if result.get("doc_id"):
|
||||
tool_event["doc_id"] = result["doc_id"]
|
||||
tool_event["doc_title"] = result.get("title", "")
|
||||
# Persist the file-write/edit diff so it re-renders on reload — without
|
||||
# this the diff shows live but vanishes from saved history.
|
||||
if result.get("diff"):
|
||||
tool_event["diff"] = result["diff"]
|
||||
tool_events.append(tool_event)
|
||||
if block.tool_type in _VERIFIER_EFFECTFUL_TOOLS:
|
||||
_effectful_used = True
|
||||
|
||||
@@ -26,7 +26,7 @@ MAX_OUTPUT_CHARS = 10_000
|
||||
MAX_READ_CHARS = 20_000
|
||||
|
||||
# Tool types that trigger execution
|
||||
TOOL_TAGS = {"bash", "python", "web_search", "web_fetch", "read_file", "write_file",
|
||||
TOOL_TAGS = {"bash", "python", "web_search", "web_fetch", "read_file", "write_file", "edit_file",
|
||||
"create_document", "update_document", "edit_document",
|
||||
"search_chats",
|
||||
"chat_with_model", "create_session", "list_sessions",
|
||||
|
||||
@@ -20,6 +20,108 @@ from src.tool_security import is_public_blocked_tool, owner_is_admin_or_single_u
|
||||
|
||||
MAX_OUTPUT_CHARS = 10_000
|
||||
MAX_READ_CHARS = 20_000
|
||||
MAX_DIFF_LINES = 400 # cap unified-diff size returned to the UI
|
||||
|
||||
|
||||
def _unified_diff(old: str, new: str, path: str) -> Optional[Dict[str, Any]]:
|
||||
"""Build a unified diff of a file write for display in the chat.
|
||||
|
||||
Returns {"text": <unified diff>, "added": N, "removed": M, "new_file": bool}
|
||||
or None when there's no textual change. Truncates very large diffs.
|
||||
"""
|
||||
if old == new:
|
||||
return None
|
||||
import difflib
|
||||
|
||||
old_lines = old.splitlines()
|
||||
new_lines = new.splitlines()
|
||||
label = path or "file"
|
||||
diff_lines = list(difflib.unified_diff(
|
||||
old_lines, new_lines,
|
||||
fromfile=f"a/{label}", tofile=f"b/{label}",
|
||||
lineterm="",
|
||||
))
|
||||
added = sum(1 for l in diff_lines if l.startswith("+") and not l.startswith("+++"))
|
||||
removed = sum(1 for l in diff_lines if l.startswith("-") and not l.startswith("---"))
|
||||
truncated = False
|
||||
if len(diff_lines) > MAX_DIFF_LINES:
|
||||
diff_lines = diff_lines[:MAX_DIFF_LINES]
|
||||
truncated = True
|
||||
text = "\n".join(diff_lines)
|
||||
if truncated:
|
||||
text += f"\n… diff truncated at {MAX_DIFF_LINES} lines"
|
||||
return {
|
||||
"text": text,
|
||||
"added": added,
|
||||
"removed": removed,
|
||||
"new_file": old == "",
|
||||
"file": os.path.basename(path) or (path or "file"),
|
||||
}
|
||||
|
||||
|
||||
async def _do_edit_file(content: str) -> Dict[str, Any]:
|
||||
"""Exact string-replacement edit of an on-disk file.
|
||||
|
||||
content is JSON: {"path", "old_string", "new_string", "replace_all"?}.
|
||||
Fails if old_string is missing or non-unique (unless replace_all) so the
|
||||
model can't silently edit the wrong place. Returns a unified diff for the UI.
|
||||
"""
|
||||
try:
|
||||
args = json.loads(content) if content.strip().startswith("{") else {}
|
||||
except (json.JSONDecodeError, TypeError):
|
||||
args = {}
|
||||
raw_path = (args.get("path") or "").strip()
|
||||
old = args.get("old_string", "")
|
||||
new = args.get("new_string", "")
|
||||
replace_all = bool(args.get("replace_all", False))
|
||||
if not raw_path:
|
||||
return {"error": "edit_file: path required", "exit_code": 1}
|
||||
# Confine to the same allowlist + sensitive-file policy as read/write_file.
|
||||
try:
|
||||
path = _resolve_tool_path(raw_path)
|
||||
except ValueError as e:
|
||||
return {"error": f"edit_file: {e}", "exit_code": 1}
|
||||
if old == "":
|
||||
return {"error": "edit_file: old_string required (use write_file to create a file)", "exit_code": 1}
|
||||
if old == new:
|
||||
return {"error": "edit_file: old_string and new_string are identical", "exit_code": 1}
|
||||
|
||||
def _apply():
|
||||
with open(path, "r", encoding="utf-8") as f:
|
||||
original = f.read()
|
||||
count = original.count(old)
|
||||
if count == 0:
|
||||
return original, None, "not_found"
|
||||
if count > 1 and not replace_all:
|
||||
return original, None, f"not_unique:{count}"
|
||||
updated = original.replace(old, new) if replace_all else original.replace(old, new, 1)
|
||||
with open(path, "w", encoding="utf-8") as f:
|
||||
f.write(updated)
|
||||
return original, updated, "ok"
|
||||
|
||||
try:
|
||||
original, updated, status = await asyncio.to_thread(_apply)
|
||||
except FileNotFoundError:
|
||||
return {"error": f"edit_file: {path}: not found (use write_file to create it)", "exit_code": 1}
|
||||
except (IsADirectoryError, UnicodeDecodeError):
|
||||
return {"error": f"edit_file: {path}: not an editable text file", "exit_code": 1}
|
||||
except PermissionError:
|
||||
return {"error": f"edit_file: {path}: permission denied", "exit_code": 1}
|
||||
except OSError as e:
|
||||
return {"error": f"edit_file: {path}: {e}", "exit_code": 1}
|
||||
|
||||
if status == "not_found":
|
||||
return {"error": f"edit_file: old_string not found in {path}. Read the file and match it exactly.", "exit_code": 1}
|
||||
if status.startswith("not_unique"):
|
||||
n = status.split(":", 1)[1]
|
||||
return {"error": f"edit_file: old_string is not unique in {path} ({n} matches). Add surrounding context or set replace_all=true.", "exit_code": 1}
|
||||
|
||||
n = original.count(old)
|
||||
result = {"output": f"Edited {path} ({n} replacement{'s' if n != 1 else ''})", "exit_code": 0}
|
||||
diff = _unified_diff(original, updated, path)
|
||||
if diff:
|
||||
result["diff"] = diff
|
||||
return result
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Path confinement for read_file / write_file
|
||||
@@ -544,18 +646,30 @@ async def _direct_fallback(
|
||||
return {"error": f"write_file: {e}", "exit_code": 1}
|
||||
try:
|
||||
def _write():
|
||||
# Capture prior content (best-effort, text) so we can show a
|
||||
# before/after diff. Missing/binary file → treat as empty.
|
||||
old = ""
|
||||
try:
|
||||
with open(path, "r", encoding="utf-8") as f:
|
||||
old = f.read()
|
||||
except (FileNotFoundError, IsADirectoryError, UnicodeDecodeError, OSError):
|
||||
old = ""
|
||||
d = os.path.dirname(path)
|
||||
if d:
|
||||
os.makedirs(d, exist_ok=True)
|
||||
with open(path, "w", encoding="utf-8") as f:
|
||||
f.write(body)
|
||||
return len(body)
|
||||
size = await asyncio.to_thread(_write)
|
||||
return old, len(body)
|
||||
old_content, size = await asyncio.to_thread(_write)
|
||||
except PermissionError:
|
||||
return {"error": f"write_file: {path}: permission denied", "exit_code": 1}
|
||||
except OSError as e:
|
||||
return {"error": f"write_file: {path}: {e}", "exit_code": 1}
|
||||
return {"output": f"Wrote {size} bytes to {path}", "exit_code": 0}
|
||||
diff = _unified_diff(old_content, body, path)
|
||||
result = {"output": f"Wrote {size} bytes to {path}", "exit_code": 0}
|
||||
if diff:
|
||||
result["diff"] = diff
|
||||
return result
|
||||
|
||||
if tool == "web_search":
|
||||
from src.search import comprehensive_web_search
|
||||
@@ -894,6 +1008,9 @@ async def execute_tool_block(
|
||||
elif tool == "edit_image":
|
||||
desc = "edit_image"
|
||||
result = await do_edit_image(content, owner=owner)
|
||||
elif tool == "edit_file":
|
||||
result = await _do_edit_file(content)
|
||||
desc = result.get("output") or result.get("error") or "edit_file"
|
||||
elif tool == "trigger_research":
|
||||
desc = "trigger_research"
|
||||
result = await do_trigger_research(content, owner=owner)
|
||||
|
||||
@@ -64,7 +64,8 @@ BUILTIN_TOOL_DESCRIPTIONS: Dict[str, str] = {
|
||||
"web_search": "Quick single web lookup for a fact, current event, or doc mid-task. NOT for 'research X' / 'do research on X' requests — those are deep-research jobs (use trigger_research). web_search = one query; trigger_research = a full researched report in the sidebar.",
|
||||
"web_fetch": "Fetch and read the text content of a specific URL/website the user names (e.g. 'check example.com', 'open this link'). Use when you have a concrete URL; for open-ended lookups use web_search instead.",
|
||||
"read_file": "Read a file from disk and return its contents. View source code, config files, logs.",
|
||||
"write_file": "Write content to a file on disk. Create new files, save output, update configs.",
|
||||
"write_file": "Write/create or fully rewrite a file ON DISK (source code, configs, project files). Use for new files or full rewrites — NOT create_document (editor panel) and NOT a bash heredoc.",
|
||||
"edit_file": "Edit an existing file ON DISK by exact string replacement (fix a bug, change a function). Shows a diff. The tool for changing files on disk — NOT edit_document (editor panel) and NOT bash sed/heredoc.",
|
||||
"create_document": "Create a new document in the editor panel. For code, articles, text content longer than 15 lines, unless an already-open document/email draft is the obvious target. If an email compose draft is open, edit that draft instead of creating another document.",
|
||||
"edit_document": "Preferred tool for editing an existing document — targeted find-and-replace. Use for any small change: add a function, fix a bug, tweak a section, rename things.",
|
||||
"update_document": "Replace the entire active document content. ONLY for full rewrites (>50% changed). Do not use for small edits — use edit_document instead.",
|
||||
|
||||
@@ -107,6 +107,23 @@ FUNCTION_TOOL_SCHEMAS = [
|
||||
}
|
||||
}
|
||||
},
|
||||
{
|
||||
"type": "function",
|
||||
"function": {
|
||||
"name": "edit_file",
|
||||
"description": "Edit a file ON DISK by exact string replacement (home folder, project files, any real path like ~/sweden.txt or /path/to/file). This is the right tool for files on disk — NOT edit_document (that's for editor-panel documents). PREFER this over bash (sed/echo) — it shows a diff. old_string must match the file exactly and be unique (or set replace_all). Use write_file to create a new file.",
|
||||
"parameters": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
"path": {"type": "string", "description": "File path to edit"},
|
||||
"old_string": {"type": "string", "description": "Exact text to replace (must match the file, including indentation)"},
|
||||
"new_string": {"type": "string", "description": "Replacement text"},
|
||||
"replace_all": {"type": "boolean", "description": "Replace all occurrences instead of requiring a unique match"}
|
||||
},
|
||||
"required": ["path", "old_string", "new_string"]
|
||||
}
|
||||
}
|
||||
},
|
||||
{
|
||||
"type": "function",
|
||||
"function": {
|
||||
@@ -127,7 +144,7 @@ FUNCTION_TOOL_SCHEMAS = [
|
||||
"type": "function",
|
||||
"function": {
|
||||
"name": "edit_document",
|
||||
"description": "PREFERRED way to change an existing document. Targeted find-and-replace with multiple FIND/REPLACE pairs per call. Use this for any edit smaller than a full rewrite: adding a function, fixing a bug, tweaking a section, renaming things. Do NOT send the whole file back via update_document for small edits — it wastes tokens and is hard to review.",
|
||||
"description": "Edit a document OPEN IN THE EDITOR PANEL (created via create_document) — NOT a file on disk. For files on disk (home folder, project files, anything with a path like ~/x.txt or /path/to/file) use edit_file instead. Targeted find-and-replace with multiple FIND/REPLACE pairs per call; use for any edit smaller than a full rewrite. Do NOT send the whole file back via update_document for small edits.",
|
||||
"parameters": {
|
||||
"type": "object",
|
||||
"properties": {
|
||||
@@ -1114,6 +1131,8 @@ def function_call_to_tool_block(name: str, arguments: str) -> Optional[ToolBlock
|
||||
content = args.get("path", "")
|
||||
elif tool_type == "write_file":
|
||||
content = args.get("path", "") + "\n" + args.get("content", "")
|
||||
elif tool_type == "edit_file":
|
||||
content = json.dumps(args)
|
||||
elif tool_type == "create_document":
|
||||
parts = [args.get("title", "Untitled")]
|
||||
if args.get("language"):
|
||||
|
||||
@@ -16,6 +16,7 @@ NON_ADMIN_BLOCKED_TOOLS = {
|
||||
"python",
|
||||
"read_file",
|
||||
"write_file",
|
||||
"edit_file",
|
||||
"search_chats",
|
||||
"manage_memory",
|
||||
"manage_skills",
|
||||
|
||||
@@ -2264,7 +2264,7 @@
|
||||
<script type="module" src="/static/js/chatRenderer.js"></script>
|
||||
<script type="module" src="/static/js/codeRunner.js"></script>
|
||||
<script type="module" src="/static/js/chatStream.js"></script>
|
||||
<script type="module" src="/static/js/chat.js?v=20260520m"></script>
|
||||
<script type="module" src="/static/js/chat.js?v=20260603n"></script>
|
||||
<script type="module" src="/static/js/cookbook.js"></script>
|
||||
<script type="module" src="/static/js/search-chat.js"></script>
|
||||
<script type="module" src="/static/js/compare/index.js"></script>
|
||||
|
||||
@@ -2074,7 +2074,33 @@ import createResearchSynapse from './researchSynapse.js';
|
||||
if (json.output && json.output.trim()) {
|
||||
outHtml = `<details class="agent-tool-output"><summary>Output</summary><pre>${esc(json.output)}</pre></details>`;
|
||||
}
|
||||
const cmdHtml2 = cmd ? `<pre class="agent-thread-cmd">${esc(cmd)}</pre>` : '';
|
||||
// File-write diff (write_file): show a before/after unified diff.
|
||||
let diffHtml = '';
|
||||
if (json.diff && json.diff.text) {
|
||||
const d = json.diff;
|
||||
// Collapsed summary: filename + +adds (green) / −dels (red).
|
||||
const stat = [
|
||||
d.new_file ? '<span class="diff-stat-new">new</span>' : '',
|
||||
d.added ? `<span class="diff-stat-add">+${d.added}</span>` : '',
|
||||
d.removed ? `<span class="diff-stat-del">−${d.removed}</span>` : '',
|
||||
].filter(Boolean).join(' ');
|
||||
const rows = d.text.split('\n').map(line => {
|
||||
let cls = 'diff-ctx', text = line;
|
||||
if (line.startsWith('+++') || line.startsWith('---')) cls = 'diff-meta';
|
||||
else if (line.startsWith('@@')) cls = 'diff-hunk';
|
||||
// Drop the leading diff marker (+/-/space) — the row colour
|
||||
// already encodes add/del, and keeping it doubles up with
|
||||
// markdown "- " bullets (reads as "+-"/"--").
|
||||
else if (line.startsWith('+')) { cls = 'diff-add'; text = line.slice(1); }
|
||||
else if (line.startsWith('-')) { cls = 'diff-del'; text = line.slice(1); }
|
||||
else if (line.startsWith(' ')) { text = line.slice(1); }
|
||||
return `<span class="${cls}">${esc(text) || ' '}</span>`;
|
||||
}).join(''); // spans are display:block — a literal \n here would double-space the diff
|
||||
diffHtml = `<details class="agent-tool-output agent-tool-diff"><summary><span class="diff-file">${esc(d.file || 'diff')}</span> <span class="diff-summary-stats">${stat}</span></summary><pre class="diff-pre">${rows}</pre></details>`;
|
||||
}
|
||||
// For file edits the "command" is the raw JSON args — redundant
|
||||
// next to the diff, so hide it when we have a diff to show.
|
||||
const cmdHtml2 = (cmd && !(json.diff && json.diff.text)) ? `<pre class="agent-thread-cmd">${esc(cmd)}</pre>` : '';
|
||||
// Preserve the user's .open choice across the innerHTML
|
||||
// rewrite \u2014 otherwise expanding a running tool collapses
|
||||
// it as soon as the result lands, forcing the user to
|
||||
@@ -2082,7 +2108,7 @@ import createResearchSynapse from './researchSynapse.js';
|
||||
// bottom of file) so no per-node listener needed.
|
||||
const _wasOpen = currentToolBubble.classList.contains('open');
|
||||
currentToolBubble.className = 'agent-thread-node' + (ok ? '' : ' error') + (_wasOpen ? ' open' : '');
|
||||
currentToolBubble.innerHTML = `<div class="agent-thread-dot"></div><div class="agent-thread-header"><span class="agent-thread-icon">${ok ? '\u2713' : '\u2717'}</span><span class="agent-thread-tool">${esc(json.tool)}</span><span class="agent-thread-status">${ok ? 'done' : 'failed'}</span><span class="agent-thread-chevron">\u25B6</span></div><div class="agent-thread-content">${cmdHtml2}${outHtml}</div>`;
|
||||
currentToolBubble.innerHTML = `<div class="agent-thread-dot"></div><div class="agent-thread-header"><span class="agent-thread-icon">${ok ? '\u2713' : '\u2717'}</span><span class="agent-thread-tool">${esc(json.tool)}</span><span class="agent-thread-status">${ok ? 'done' : 'failed'}</span><span class="agent-thread-chevron">\u25B6</span></div><div class="agent-thread-content">${cmdHtml2}${outHtml}${diffHtml}</div>`;
|
||||
// Reset so thinking spinner between tools says "Thinking" not the old tool's label
|
||||
_lastToolName = '';
|
||||
uiModule.scrollHistory();
|
||||
|
||||
@@ -1956,10 +1956,33 @@ export function addMessage(role, content, modelName, metadata) {
|
||||
if (ev.screenshot) {
|
||||
outHtml += `<details class="agent-tool-output"><summary>Screenshot</summary><img src="${esc(ev.screenshot)}" style="max-width:100%;border-radius:6px;margin-top:6px;border:1px solid var(--border)" /></details>`;
|
||||
}
|
||||
// File-write/edit diff (persisted in the tool event) \u2014 re-render it
|
||||
// so it survives reload, matching the live stream.
|
||||
let evDiffHtml = '';
|
||||
if (ev.diff && ev.diff.text) {
|
||||
const d = ev.diff;
|
||||
const stat = [
|
||||
d.new_file ? '<span class="diff-stat-new">new</span>' : '',
|
||||
d.added ? `<span class="diff-stat-add">+${d.added}</span>` : '',
|
||||
d.removed ? `<span class="diff-stat-del">\u2212${d.removed}</span>` : '',
|
||||
].filter(Boolean).join(' ');
|
||||
const rows = d.text.split('\n').map(line => {
|
||||
let cls = 'diff-ctx', text = line;
|
||||
if (line.startsWith('+++') || line.startsWith('---')) cls = 'diff-meta';
|
||||
else if (line.startsWith('@@')) cls = 'diff-hunk';
|
||||
// Drop the leading diff marker (+/-/space) — colour encodes add/del.
|
||||
else if (line.startsWith('+')) { cls = 'diff-add'; text = line.slice(1); }
|
||||
else if (line.startsWith('-')) { cls = 'diff-del'; text = line.slice(1); }
|
||||
else if (line.startsWith(' ')) { text = line.slice(1); }
|
||||
return `<span class="${cls}">${esc(text) || ' '}</span>`;
|
||||
}).join(''); // spans are display:block \u2014 a literal \n would double-space
|
||||
evDiffHtml = `<details class="agent-tool-output agent-tool-diff"><summary><span class="diff-file">${esc(d.file || 'diff')}</span> <span class="diff-summary-stats">${stat}</span></summary><pre class="diff-pre">${rows}</pre></details>`;
|
||||
}
|
||||
const node = document.createElement('div');
|
||||
node.className = 'agent-thread-node' + (ok ? '' : ' error');
|
||||
const evCmdHtml = ev.command ? `<pre class="agent-thread-cmd">${esc(ev.command)}</pre>` : '';
|
||||
node.innerHTML = `<div class="agent-thread-dot"></div><div class="agent-thread-header"><span class="agent-thread-icon">${ok ? '\u2713' : '\u2717'}</span><span class="agent-thread-tool">${esc(ev.tool)}</span><span class="agent-thread-status">${ok ? 'done' : 'failed'}</span><span class="agent-thread-chevron">\u25B6</span></div><div class="agent-thread-content">${evCmdHtml}${outHtml}</div>`;
|
||||
// Hide the raw JSON command when a diff says it better (same as live).
|
||||
const evCmdHtml = (ev.command && !(ev.diff && ev.diff.text)) ? `<pre class="agent-thread-cmd">${esc(ev.command)}</pre>` : '';
|
||||
node.innerHTML = `<div class="agent-thread-dot"></div><div class="agent-thread-header"><span class="agent-thread-icon">${ok ? '\u2713' : '\u2717'}</span><span class="agent-thread-tool">${esc(ev.tool)}</span><span class="agent-thread-status">${ok ? 'done' : 'failed'}</span><span class="agent-thread-chevron">\u25B6</span></div><div class="agent-thread-content">${evCmdHtml}${outHtml}${evDiffHtml}</div>`;
|
||||
// Click handling is delegated globally \u2014 see chat.js init.
|
||||
threadWrap.appendChild(node);
|
||||
}
|
||||
|
||||
@@ -8835,6 +8835,57 @@ body.hide-thinking .thinking-section { display: none !important; }
|
||||
list-style: none;
|
||||
}
|
||||
.agent-tool-output summary::-webkit-details-marker { display: none; }
|
||||
/* File-write diff — neutral chrome (not the red error tint) + colored lines */
|
||||
.agent-tool-diff {
|
||||
background: color-mix(in srgb, var(--fg) 4%, transparent);
|
||||
border-color: color-mix(in srgb, var(--fg) 18%, transparent);
|
||||
}
|
||||
.agent-tool-diff summary {
|
||||
color: var(--fg);
|
||||
background: color-mix(in srgb, var(--fg) 7%, transparent);
|
||||
border-bottom-color: color-mix(in srgb, var(--fg) 12%, transparent);
|
||||
}
|
||||
.agent-tool-diff .diff-stat {
|
||||
font-weight: 600;
|
||||
opacity: 0.7;
|
||||
font-family: var(--mono, monospace);
|
||||
}
|
||||
/* Collapsed diff summary: filename + +adds/−dels (theme green/red). */
|
||||
.agent-tool-diff summary {
|
||||
display: flex;
|
||||
align-items: center;
|
||||
gap: 8px;
|
||||
}
|
||||
.agent-tool-diff .diff-file {
|
||||
font-family: var(--mono, monospace);
|
||||
font-weight: 600;
|
||||
overflow: hidden;
|
||||
text-overflow: ellipsis;
|
||||
white-space: nowrap;
|
||||
}
|
||||
.agent-tool-diff .diff-summary-stats {
|
||||
margin-left: auto;
|
||||
font-family: var(--mono, monospace);
|
||||
font-weight: 600;
|
||||
flex-shrink: 0;
|
||||
}
|
||||
.agent-tool-diff .diff-summary-stats .diff-stat-add { color: var(--green, #2ecc71); }
|
||||
.agent-tool-diff .diff-summary-stats .diff-stat-del { color: var(--red, #e74c3c); }
|
||||
.agent-tool-diff .diff-summary-stats .diff-stat-new { color: var(--accent, var(--red)); opacity: 0.85; }
|
||||
.diff-pre {
|
||||
margin: 0;
|
||||
padding: 8px 10px;
|
||||
overflow-x: auto;
|
||||
font-family: var(--mono, monospace);
|
||||
font-size: 0.82em;
|
||||
line-height: 1.45;
|
||||
}
|
||||
.diff-pre span { display: block; white-space: pre; }
|
||||
.diff-pre .diff-add { background: color-mix(in srgb, #2ecc71 22%, transparent); }
|
||||
.diff-pre .diff-del { background: color-mix(in srgb, #e74c3c 22%, transparent); }
|
||||
.diff-pre .diff-hunk { color: var(--accent); opacity: 0.85; }
|
||||
.diff-pre .diff-meta { opacity: 0.55; }
|
||||
.diff-pre .diff-ctx { opacity: 0.8; }
|
||||
/* Suppress the global `summary::before { content: '▶' }` left arrow — this
|
||||
section uses a right-side chevron instead. */
|
||||
.agent-tool-output summary::before { content: none; }
|
||||
|
||||
87
tests/test_edit_file.py
Normal file
87
tests/test_edit_file.py
Normal file
@@ -0,0 +1,87 @@
|
||||
"""edit_file: filesystem-write permission policy + behavior."""
|
||||
import json
|
||||
import os
|
||||
import tempfile
|
||||
|
||||
import pytest
|
||||
|
||||
from src import tool_security
|
||||
from src.tool_security import (
|
||||
NON_ADMIN_BLOCKED_TOOLS,
|
||||
is_public_blocked_tool,
|
||||
blocked_tools_for_owner,
|
||||
)
|
||||
from src.tool_execution import _do_edit_file, execute_tool_block
|
||||
from src.agent_tools import ToolBlock
|
||||
|
||||
|
||||
# ── Permission policy ─────────────────────────────────────────────────────
|
||||
def test_edit_file_is_sensitive_write_tool():
|
||||
# Must be blocked for non-admins exactly like write_file.
|
||||
assert "edit_file" in NON_ADMIN_BLOCKED_TOOLS
|
||||
assert is_public_blocked_tool("edit_file") is True
|
||||
|
||||
|
||||
def test_blocked_tools_for_owner_includes_edit_file_for_non_admin(monkeypatch):
|
||||
monkeypatch.setattr(tool_security, "owner_is_admin_or_single_user", lambda owner: False)
|
||||
blocked = blocked_tools_for_owner("bob")
|
||||
assert "edit_file" in blocked and "write_file" in blocked
|
||||
# Admin / single-user gets nothing blocked.
|
||||
monkeypatch.setattr(tool_security, "owner_is_admin_or_single_user", lambda owner: True)
|
||||
assert blocked_tools_for_owner("admin") == set()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_edit_file_blocked_at_execution_for_non_admin(monkeypatch):
|
||||
# Execution-level gate: a non-admin owner must be refused even if the tool
|
||||
# reaches execute_tool_block.
|
||||
import src.tool_execution as te
|
||||
monkeypatch.setattr(te, "_owner_is_admin", lambda owner: False)
|
||||
ws = tempfile.mkdtemp()
|
||||
p = os.path.join("/tmp", "ef_block.txt")
|
||||
open(p, "w").write("a\n")
|
||||
_desc, result = await execute_tool_block(
|
||||
ToolBlock("edit_file", json.dumps({"path": p, "old_string": "a", "new_string": "b"})),
|
||||
owner="bob",
|
||||
)
|
||||
assert result.get("exit_code") == 1 and "admin" in result.get("error", "").lower()
|
||||
os.unlink(p)
|
||||
|
||||
|
||||
# ── Behavior ──────────────────────────────────────────────────────────────
|
||||
@pytest.mark.asyncio
|
||||
async def test_edit_file_success():
|
||||
p = os.path.join("/tmp", "ef_ok.py")
|
||||
open(p, "w").write("def f():\n return 1\n")
|
||||
res = await _do_edit_file(json.dumps({"path": p, "old_string": "return 1", "new_string": "return 2"}))
|
||||
assert res["exit_code"] == 0
|
||||
assert open(p).read() == "def f():\n return 2\n"
|
||||
assert res["diff"]["added"] == 1 and res["diff"]["removed"] == 1 and res["diff"]["file"] == "ef_ok.py"
|
||||
os.unlink(p)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_edit_file_not_found():
|
||||
p = os.path.join("/tmp", "ef_nf.txt")
|
||||
open(p, "w").write("hello\n")
|
||||
res = await _do_edit_file(json.dumps({"path": p, "old_string": "nope", "new_string": "x"}))
|
||||
assert res["exit_code"] == 1 and "not found" in res["error"]
|
||||
os.unlink(p)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_edit_file_non_unique():
|
||||
p = os.path.join("/tmp", "ef_dup.txt")
|
||||
open(p, "w").write("x\nx\n")
|
||||
res = await _do_edit_file(json.dumps({"path": p, "old_string": "x", "new_string": "y"}))
|
||||
assert res["exit_code"] == 1 and "not unique" in res["error"]
|
||||
# replace_all resolves it
|
||||
res = await _do_edit_file(json.dumps({"path": p, "old_string": "x", "new_string": "y", "replace_all": True}))
|
||||
assert res["exit_code"] == 0 and open(p).read() == "y\ny\n"
|
||||
os.unlink(p)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_edit_file_outside_allowed_roots():
|
||||
res = await _do_edit_file(json.dumps({"path": "/etc/hosts", "old_string": "x", "new_string": "y"}))
|
||||
assert res["exit_code"] == 1 and ("outside the allowed roots" in res["error"] or "sensitive" in res["error"])
|
||||
Reference in New Issue
Block a user