diff --git a/src/agent_loop.py b/src/agent_loop.py index 653baa9..d6d9370 100644 --- a/src/agent_loop.py +++ b/src/agent_loop.py @@ -177,6 +177,7 @@ TOOL_SECTIONS = { ``` 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": "", "old_string": "", "new_string": "", "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 @@ -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 diff --git a/src/agent_tools.py b/src/agent_tools.py index 2785623..f93df01 100644 --- a/src/agent_tools.py +++ b/src/agent_tools.py @@ -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", diff --git a/src/tool_execution.py b/src/tool_execution.py index b0e8e2d..626bf5d 100644 --- a/src/tool_execution.py +++ b/src/tool_execution.py @@ -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) diff --git a/src/tool_index.py b/src/tool_index.py index 3c5150e..04435ad 100644 --- a/src/tool_index.py +++ b/src/tool_index.py @@ -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.", diff --git a/src/tool_schemas.py b/src/tool_schemas.py index b862301..05134ae 100644 --- a/src/tool_schemas.py +++ b/src/tool_schemas.py @@ -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"): diff --git a/src/tool_security.py b/src/tool_security.py index c4094b9..dd2ce83 100644 --- a/src/tool_security.py +++ b/src/tool_security.py @@ -16,6 +16,7 @@ NON_ADMIN_BLOCKED_TOOLS = { "python", "read_file", "write_file", + "edit_file", "search_chats", "manage_memory", "manage_skills", diff --git a/static/index.html b/static/index.html index 72544de..fadb0c6 100644 --- a/static/index.html +++ b/static/index.html @@ -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> diff --git a/static/js/chat.js b/static/js/chat.js index ee347b9..2415f40 100644 --- a/static/js/chat.js +++ b/static/js/chat.js @@ -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(); diff --git a/static/js/chatRenderer.js b/static/js/chatRenderer.js index 93e6a7d..8780864 100644 --- a/static/js/chatRenderer.js +++ b/static/js/chatRenderer.js @@ -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); } diff --git a/static/style.css b/static/style.css index fcb607f..69e02e7 100644 --- a/static/style.css +++ b/static/style.css @@ -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; } diff --git a/tests/test_edit_file.py b/tests/test_edit_file.py new file mode 100644 index 0000000..23c5f2b --- /dev/null +++ b/tests/test_edit_file.py @@ -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"])