From 8ca905799bdaad9556b5bb6331252185f70fd488 Mon Sep 17 00:00:00 2001 From: Levi Neely Date: Tue, 14 Apr 2026 22:52:10 +0200 Subject: [PATCH] fix: error handling, sandbox env, file tool enforcement MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - tools/file_*: hard-fail when OLLIE_SESSION_ID is unset; surface errors on all failure paths including dir checks, marker cleanup, and I/O - tools/file_read: treat nonexistent path as new file (emit "(new file)", write empty marker) so file_write can create new files - tools/file_write: colorize new-file lines with ＋ (U+FF0B) - tools/file_glob, file_grep, memory_*, task_*: consistent error handling and output to stdout so the model sees all failures - tui/diffcolor.go: match ＋/− (U+FF0B/U+2212) lookalikes emitted by file_edit/file_write; fix +++ / --- header pattern - prompts: prohibit execute_code for file creation/modification; enforce execute_tool with file_write/file_edit instead - core submodule: surface isError from MCP responses; inject OLLIE_SESSION_ID into execute server at session creation Co-Authored-By: Claude Sonnet 4.6 --- core | 2 +- prompts/00_base.md | 3 +++ prompts/07_edit-text.md | 2 +- tools/file_edit | 23 ++++++++++++++++++++--- tools/file_glob | 19 +++++++++++++++++-- tools/file_grep | 5 +++-- tools/file_read | 19 +++++++++++++++---- tools/file_write | 18 +++++++++++++++--- tools/memory_recall | 14 ++++++++++++-- tools/memory_remember | 4 ++++ tools/task_complete | 30 +++++++++++++++++++++++------- tools/task_plan | 11 ++++++++--- 12 files changed, 122 insertions(+), 28 deletions(-) diff --git a/core b/core index 64d3915..655b9e7 160000 --- a/core +++ b/core @@ -1 +1 @@ -Subproject commit 64d3915593c9600911cf332b9f0d42bc8a4e2d5d +Subproject commit 655b9e706045f503b4889c6bee6e43261b50f4f0 diff --git a/prompts/00_base.md b/prompts/00_base.md index 196a46e..860f14f 100644 --- a/prompts/00_base.md +++ b/prompts/00_base.md @@ -1,5 +1,7 @@ You are ollie, an agentic assistant. +**Critical rule: never use `execute_code` to create or modify files. Always use `execute_tool` with `file_write` or `file_edit`. No exceptions.** + # Tone and style - Format output as markdown. The frontend may render it as HTML, display it in a terminal, or show it as plain text — never rely on ANSI escape codes or terminal-specific formatting. @@ -20,6 +22,7 @@ Be truthful. If the user is wrong, say so — do not soften, hedge, or dance aro # Tool preferences - Make independent tool calls in parallel when there are no dependencies between them. +- **Never use `execute_code` to create or modify files.** Use `execute_tool` with `file_write` or `file_edit` instead. This is a hard rule — there are no exceptions for "simple" files. # Environment diff --git a/prompts/07_edit-text.md b/prompts/07_edit-text.md index 403b83c..52b1ad2 100644 --- a/prompts/07_edit-text.md +++ b/prompts/07_edit-text.md @@ -18,7 +18,7 @@ Using `cat`, `sed`, `awk`, `grep`, `find`, `echo >`, or similar shell commands d - **Read before writing.** Always call `file_read` on a file before `file_write` or `file_edit`. This is enforced — writes will fail without a prior read. - **Prefer `file_edit` over `file_write`** for partial changes. -- **Never use `execute_code` or `execute_tool` for file I/O.** Reserve those for operations that genuinely require a shell: building, testing, git, process control. +- **To create or modify a file with content you provide, always use `execute_tool` with `file_write` or `file_edit` — never `execute_code`.** Capturing process output to a file (e.g. `make > make.log`, `openssl genrsa > key.pem`) is fine. ## Examples diff --git a/tools/file_edit b/tools/file_edit index 100ec53..75e1ad9 100755 --- a/tools/file_edit +++ b/tools/file_edit @@ -8,14 +8,27 @@ import hashlib import difflib def marker_path(file_path): + sid = os.environ.get('OLLIE_SESSION_ID', '') + if not sid: + print("error: OLLIE_SESSION_ID is not set") + sys.exit(1) h = hashlib.sha256(file_path.encode()).hexdigest()[:64] - sid = os.environ.get('OLLIE_SESSION_ID', '_') return f"/tmp/ollie/{sid}/{h}" def unified_diff(path, old, new): old_lines = old.splitlines(keepends=True) new_lines = new.splitlines(keepends=True) - return ''.join(difflib.unified_diff(old_lines, new_lines, fromfile=path, tofile=path)) + raw = ''.join(difflib.unified_diff(old_lines, new_lines, fromfile=path, tofile=path)) + # Replace leading +/- with Unicode lookalikes so only the colorizer's + # explicit patterns trigger, not incidental ASCII +/- in content. + out = [] + for line in raw.splitlines(keepends=True): + if line.startswith('+'): + line = '＋' + line[1:] + elif line.startswith('-'): + line = '−' + line[1:] + out.append(line) + return ''.join(out) def find_exact(content, find): return find if find in content else None @@ -124,7 +137,11 @@ except OSError as e: print(f"error: {e}") sys.exit(1) -os.unlink(marker) +try: + os.unlink(marker) +except OSError as e: + print(f"error: failed to remove marker: {e}") + sys.exit(1) diff = unified_diff(file_path, original, new_content) print(diff if diff else "(no changes)", end='' if diff else '\n') diff --git a/tools/file_glob b/tools/file_glob index 3daf256..d5b3dd1 100755 --- a/tools/file_glob +++ b/tools/file_glob @@ -7,8 +7,23 @@ search_path="${2:-$PWD}" [ -z "$pattern" ] && echo "usage: file_glob [path]" && exit 1 +if [ ! -d "$search_path" ]; then + echo "error: path is not a directory: $search_path" + exit 1 +fi + # collect matches and sort by mtime descending -rg --no-config --files -g "$pattern" "$search_path" 2>/dev/null \ - | xargs -d '\n' stat --printf '%Y\t%n\n' 2>/dev/null \ +rg_out=$(rg --no-config --files -g "$pattern" "$search_path" 2>&1) +rg_exit=$? +if [ $rg_exit -gt 1 ]; then + echo "error: $rg_out" + exit 1 +fi +if [ -z "$rg_out" ]; then + echo "no matches" + exit 0 +fi +echo "$rg_out" \ + | xargs -d '\n' stat --printf '%Y\t%n\n' 2>&1 \ | sort -rn \ | cut -f2- diff --git a/tools/file_grep b/tools/file_grep index 4627781..7599058 100755 --- a/tools/file_grep +++ b/tools/file_grep @@ -57,9 +57,10 @@ cmd += ['-e', args.pattern, search_path] result = subprocess.run(cmd, capture_output=True, text=True) if result.returncode == 1: - sys.exit(0) # no matches + print("no matches") + sys.exit(0) if result.returncode > 1: - print(result.stderr.strip() or f"rg exited with {result.returncode}", file=sys.stderr) + print(f"error: {result.stderr.strip() or f'rg exited with {result.returncode}'}") sys.exit(1) lines = result.stdout.splitlines() diff --git a/tools/file_read b/tools/file_read index 6f46951..c1a35a7 100755 --- a/tools/file_read +++ b/tools/file_read @@ -10,8 +10,21 @@ max_size=$((8 * 1024 * 1024)) [ -z "$file_path" ] && echo "usage: file_read [offset] [limit]" && exit 1 [[ "$file_path" != /* ]] && echo "error: path must be absolute, got: $file_path" && exit 1 -[ ! -e "$file_path" ] && echo "error: file not found: $file_path" && exit 1 -[ ! -f "$file_path" ] && echo "error: not a regular file: $file_path" && exit 1 +[ ! -f "$file_path" ] && [ -e "$file_path" ] && echo "error: not a regular file: $file_path" && exit 1 + +if [ -z "$OLLIE_SESSION_ID" ]; then + echo "error: OLLIE_SESSION_ID is not set" + exit 1 +fi + +mkdir -p "/tmp/ollie/$OLLIE_SESSION_ID" +marker="/tmp/ollie/$OLLIE_SESSION_ID/$(printf '%s' "$file_path" | sha256sum | cut -c1-64)" + +if [ ! -e "$file_path" ]; then + echo "(new file)" + : > "$marker" + exit 0 +fi size=$(wc -c < "$file_path") [ "$size" -gt "$max_size" ] && echo "error: file too large ($size bytes, max $max_size)" && exit 1 @@ -31,6 +44,4 @@ else fi # save content to marker (used by file_edit/file_write as staging area) -mkdir -p "/tmp/ollie/$OLLIE_SESSION_ID" -marker="/tmp/ollie/$OLLIE_SESSION_ID/$(printf '%s' "$file_path" | sha256sum | cut -c1-64)" cp "$file_path" "$marker" diff --git a/tools/file_write b/tools/file_write index a143ebf..523a67d 100755 --- a/tools/file_write +++ b/tools/file_write @@ -8,8 +8,11 @@ import hashlib import difflib def marker_path(file_path): + sid = os.environ.get('OLLIE_SESSION_ID', '') + if not sid: + print("error: OLLIE_SESSION_ID is not set") + sys.exit(1) h = hashlib.sha256(file_path.encode()).hexdigest()[:64] - sid = os.environ.get('OLLIE_SESSION_ID', '_') return f"/tmp/ollie/{sid}/{h}" def unified_diff(path, old, new): @@ -39,6 +42,11 @@ if exists: with open(marker, 'r', errors='replace') as f: old_content = f.read() +parent = os.path.dirname(file_path) +if parent and not os.path.isdir(parent): + print(f"error: parent directory does not exist: {parent}") + sys.exit(1) + try: with open(marker, 'w') as f: f.write(content) @@ -48,11 +56,15 @@ except OSError as e: print(f"error: {e}") sys.exit(1) -os.unlink(marker) +try: + os.unlink(marker) +except OSError as e: + print(f"error: failed to remove marker: {e}") + sys.exit(1) if not exists: for line in content.splitlines(): - print(f"+{line}") + print(f"＋{line}") else: diff = unified_diff(file_path, old_content, content) print(diff if diff else "(no changes)", end='' if diff else '\n') diff --git a/tools/memory_recall b/tools/memory_recall index c212b59..7d0d780 100755 --- a/tools/memory_recall +++ b/tools/memory_recall @@ -7,10 +7,20 @@ mount="${OLLIE:-$HOME/mnt/ollie}" mem_dir="$mount/m" +if [ ! -d "$mem_dir" ]; then + echo "error: memory directory does not exist: $mem_dir" + exit 1 +fi + query="$*" # Search filenames (title/tag match) and file bodies -matches=$(grep -ril "$query" "$mem_dir" 2>/dev/null) +matches=$(grep -ril "$query" "$mem_dir" 2>&1) +grep_exit=$? +if [ $grep_exit -gt 1 ]; then + echo "error: $matches" + exit 1 +fi if [ -z "$matches" ]; then echo "no matches for: $query" @@ -19,6 +29,6 @@ fi while IFS= read -r file; do echo "=== $(basename "$file") ===" - cat "$file" + cat "$file" || echo "error: could not read $file" echo done <<< "$matches" diff --git a/tools/memory_remember b/tools/memory_remember index 5218ffe..e4f6ff8 100755 --- a/tools/memory_remember +++ b/tools/memory_remember @@ -18,6 +18,10 @@ body = sys.argv[3] mount = os.environ.get("OLLIE") or os.path.expanduser("~/mnt/ollie") mem_dir = os.path.join(mount, "m") +if not os.path.isdir(mem_dir): + print(f"error: memory directory does not exist: {mem_dir}") + sys.exit(1) + now = datetime.now() ident = now.strftime("%Y%m%dT%H%M%S") date = now.strftime("%Y-%m-%d %a %H:%M") diff --git a/tools/task_complete b/tools/task_complete index 3899aa7..3805c7d 100755 --- a/tools/task_complete +++ b/tools/task_complete @@ -4,7 +4,7 @@ import sys, os, re if len(sys.argv) < 3: - print("usage: task_complete ", file=sys.stderr) + print("usage: task_complete ", file=sys.stdout) sys.exit(1) ollie = os.environ.get('OLLIE', os.path.expanduser('~/mnt/ollie')) @@ -14,8 +14,16 @@ plan_dir = os.path.join(ollie, 'pl') filepath = os.path.join(plan_dir, os.path.basename(sys.argv[1])) substring = sys.argv[2].lower() -with open(filepath) as f: - lines = f.readlines() +if not os.path.exists(filepath): + print(f"error: plan file not found: {filepath}", file=sys.stdout) + sys.exit(1) + +try: + with open(filepath) as f: + lines = f.readlines() +except OSError as e: + print(f"error: {e}", file=sys.stdout) + sys.exit(1) matched = False for i, line in enumerate(lines): @@ -25,7 +33,7 @@ for i, line in enumerate(lines): break if not matched: - print(f"no unchecked step matching {sys.argv[2]!r}", file=sys.stderr) + print(f"no unchecked step matching {sys.argv[2]!r}", file=sys.stdout) sys.exit(1) remaining = [line[6:].strip() for line in lines if re.match(r'^- \[ \] ', line)] @@ -40,11 +48,19 @@ elif not remaining and '__wip' in filepath: else: newpath = filepath -with open(filepath, 'w') as f: - f.writelines(lines) +try: + with open(filepath, 'w') as f: + f.writelines(lines) +except OSError as e: + print(f"error: failed to write {filepath}: {e}", file=sys.stdout) + sys.exit(1) if newpath != filepath: - os.rename(filepath, newpath) + try: + os.rename(filepath, newpath) + except OSError as e: + print(f"error: failed to rename to {newpath}: {e}", file=sys.stdout) + sys.exit(1) if remaining: print('\n'.join(f'- {s}' for s in remaining)) diff --git a/tools/task_plan b/tools/task_plan index 27a3ab7..d46ffc0 100755 --- a/tools/task_plan +++ b/tools/task_plan @@ -8,14 +8,19 @@ OLLIE="${OLLIE:-$HOME/mnt/ollie}" PLAN_DIR="$OLLIE/pl" if [ $# -eq 0 ]; then - echo "usage: task_plan [step2] ..." >&2 + echo "usage: task_plan [step2] ..." exit 1 fi GOAL="$1" shift if [ $# -eq 0 ]; then - echo "error: at least one step required" >&2 + echo "error: at least one step required" + exit 1 +fi + +if [ ! -d "$PLAN_DIR" ]; then + echo "error: plan directory does not exist: $PLAN_DIR" exit 1 fi @@ -36,6 +41,6 @@ FILEPATH="$PLAN_DIR/$FILENAME" for step in "$@"; do printf '- [ ] %s\n' "$step" done -} > "$FILEPATH" +} > "$FILEPATH" || { echo "error: failed to write $FILEPATH"; exit 1; } echo "$FILENAME"