Merge pull request #646 from fabro-sh/fix/remove-read-before-write-guard

fix(agent): remove the read-before-write guard

docs/public/agents/permissions.mdx

@@ -80,15 +80,6 @@ The `shell` tool and web tools (`web_search`, `web_fetch`) are only auto-approve
 
 Sub-agent management tools (`spawn_agent`, `send_input`, `wait`, `close_agent`) are auto-approved at all permission levels. Sub-agents inherit the parent's permission level, so a `read-only` parent spawns `read-only` children.
 
-## Read-before-write guardrail
-
-Independent of the permission system, Fabro enforces a **read-before-write guardrail** that applies to all permission levels. Even with `full` permissions, an agent must read an existing file before modifying or deleting it. See [Tools: Read-before-write guardrail](/agents/tools#read-before-write-guardrail) for details.
-
-This guardrail operates at the sandbox layer and works alongside (not instead of) the permission system. A tool call must pass **both** checks:
-
-1. The tool's category must be allowed by the current permission level
-2. If writing to an existing file, the file must have been previously read
-
 ## Permission flow
 
 The following sequence shows how Fabro decides whether to execute a tool call:

docs/public/agents/tools.mdx

@@ -76,7 +76,7 @@ Creates or overwrites a file.
 | `content` | string | yes | Content to write |
 
 <Note>
-The [read-before-write guardrail](#read-before-write-guardrail) prevents writing to existing files that haven't been read first. Writing to new files is always allowed.
+`write_file` replaces the entire file. For changes to an existing file, prefer `edit_file`, which only replaces the string you name.
 </Note>
 
 ### edit_file
@@ -92,11 +92,11 @@ Replaces a string in an existing file. Available for Anthropic and Gemini provid
 
 If `old_string` is not found, the tool returns an error. If multiple occurrences exist and `replace_all` is false, the tool returns an error asking for more context or to set `replace_all`.
 
-The tool reads the file internally before writing, so it satisfies the read-before-write guardrail automatically.
+Because `old_string` must match the file exactly, an edit built from a stale or remembered version of the file fails rather than silently applying somewhere unintended.
 
 ### grep
 
-Searches file contents with a regex pattern, powered by ripgrep in the sandbox.
+Searches file contents with a regex pattern. The sandbox uses ripgrep when `rg` is on the path and falls back to POSIX `grep` otherwise, detected once and cached per sandbox. Keep patterns portable across both rather than relying on ripgrep-only syntax.
 
 | Parameter | Type | Required | Description |
 |---|---|---|---|
@@ -106,7 +106,7 @@ Searches file contents with a regex pattern, powered by ripgrep in the sandbox.
 | `case_insensitive` | boolean | no | Case-insensitive search (default: false) |
 | `max_results` | integer | no | Maximum number of results |
 
-Results are returned as `file:line:content` lines. Files that appear in grep results are marked as "read" for the [read-before-write guardrail](#read-before-write-guardrail).
+Results are returned as `file:line:content` lines.
 
 ### glob
 
@@ -121,10 +121,6 @@ Returns matching file paths, one per line, sorted lexicographically by their pat
 
 Patterns are case-sensitive and relative to `path`: `*` and `?` stay within one path segment, bracket expressions such as `[abc]` match one character, and `**` crosses directories when used as a complete segment. Leading dots are matched normally. For example, `*.rs` searches only the root of `path`, and `**/*.rs` searches recursively. Patterns must use `/`, be relative, and cannot contain a backslash or `..` segment.
 
-<Note>
-Unlike `grep`, `glob` does **not** mark files as read for the read-before-write guardrail. To modify a file found via glob, the agent must read it first.
-</Note>
-
 ### web_search
 
 Searches the web using the Brave Search API.
@@ -202,23 +198,6 @@ Like `update_plan`, task changes are persisted as `todo.created`, `todo.updated`
 
 When an Anthropic session has not used `TaskCreate` or `TaskUpdate` for ten assistant turns, Fabro may inject a system reminder asking the agent to keep task state current. The reminder is only added when both tools are available and resets after the agent uses either tool.
 
-## Read-before-write guardrail
-
-Fabro wraps every sandbox in a `ReadBeforeWriteSandbox` decorator that tracks which files the agent has seen. The rules are:
-
-1. **Writing to a new file** (one that doesn't exist yet) is always allowed
-2. **Writing to an existing file** requires that the agent has previously read it via `read_file` or seen it in `grep` results
-3. **Deleting an existing file** follows the same rule as writing
-
-If an agent attempts to write to an existing file it hasn't read, the tool returns an error:
-
-```
-Cannot write to 'src/main.rs': file exists but has not been read.
-Use read_file to read the file before writing to it.
-```
-
-This prevents agents from blindly overwriting files they haven't inspected. The guardrail normalizes paths so that reading `src/main.rs` (relative) and `/workspace/src/main.rs` (absolute) both satisfy the check.
-
 ## Tool execution
 
 ### Parallel execution
@@ -256,7 +235,7 @@ Common error cases:
 | Argument validation failure | Arguments don't match the tool's JSON Schema |
 | File not found | The target file doesn't exist |
 | Command timeout | A shell command exceeded its timeout |
-| Read-before-write | The agent tried to write to a file it hasn't read (see [guardrail](#read-before-write-guardrail)) |
+| `old_string` not found | An `edit_file` anchor didn't match the file's current contents |
 
 ### Timeouts
 

docs/public/reference/sdk.mdx

@@ -161,7 +161,6 @@ pub trait Sandbox: Send + Sync {
 |---|---|
 | `LocalSandbox` | Executes directly on the local filesystem. |
 | `DockerSandbox` | Runs inside a Docker container (feature-gated: `docker`). |
-| `ReadBeforeWriteSandbox` | Decorator that blocks writes to files the agent hasn't read. Wraps any `Arc<dyn Sandbox>`. |
 
 The `DaytonaSandbox` implementation (feature-gated: `daytona`) runs inside a Daytona cloud sandbox.
 

lib/components/fabro-agent/src/cli.rs

@@ -556,9 +556,7 @@ pub async fn run_with_args_and_client_and_catalog(
     // Build sandbox
     let cwd = std::env::current_dir().unwrap_or_else(|_| PathBuf::from("."));
     let cwd_str = cwd.to_string_lossy().to_string();
-    let env: Arc<dyn Sandbox> = Arc::new(crate::ReadBeforeWriteSandbox::new(Arc::new(
-        LocalSandbox::new(cwd),
-    )));
+    let env: Arc<dyn Sandbox> = Arc::new(LocalSandbox::new(cwd));
 
     // Build tool approval callback
     let permissions = args.permissions.unwrap_or(PermissionLevel::ReadWrite);

lib/components/fabro-agent/src/lib.rs

@@ -18,7 +18,6 @@ pub mod memory;
 pub mod native_tool;
 pub mod profiles;
 pub mod question_tools;
-pub mod read_before_write_sandbox;
 pub mod sandbox;
 pub mod session;
 pub mod skills;
@@ -57,7 +56,6 @@ pub use question_tools::{
     AgentQuestionAnswerStatus, AgentQuestionRuntime, AgentToolRuntime,
     OPENAI_REQUEST_USER_INPUT_TOOL, register_question_tools,
 };
-pub use read_before_write_sandbox::ReadBeforeWriteSandbox;
 pub use sandbox::{
     CommandOutputCallback, DirEntry, ExecResult, ExecStreamingResult, GrepOptions, RefreshOutcome,
     Sandbox, SandboxEvent, SandboxEventCallback, StderrCollector, StdioProcess, StdioProcessHandle,

lib/components/fabro-agent/src/profiles/kimi.rs

@@ -16,20 +16,28 @@ use crate::tools::{WebFetchSummarizer, register_discovery_and_web_tools};
 
 const CORE_PROMPT: &str = include_str!("prompts/kimi.md.j2");
 
-/// Kimi models repeatedly fail the workspace's read-before-write guard: across
-/// two observed K3 implementation stages, 32 of 35 tool failures were writes to
-/// files the model had not read, or `old_string` values reconstructed from
-/// memory. Kimi Code's own tool descriptions drill this rule directly, so the
-/// Kimi profile restates it where the model is most likely to act on it — in
-/// the description of the tool being called — rather than relying only on the
-/// system prompt.
-const EDIT_FILE_DESCRIPTION: &str = "Edit a file by replacing an exact string. \
-Read the file with Read before EVERY edit — this workspace refuses writes to files that \
-have not been read, and the call will fail. Take old_string verbatim from the Read output; \
-never reconstruct it from memory or from an earlier version of the file. old_string must be an \
-exact match and unique unless replace_all is true. If the edit fails with 'old_string not found', \
-re-read the file and take the exact text from the fresh output rather than guessing again. \
-Preserve existing indentation.";
+/// Kimi models repeatedly reconstruct `old_string` from memory rather than from
+/// a fresh read: across two observed K3 implementation stages, 32 of 35 tool
+/// failures were edits against a file the model had not read, or `old_string`
+/// values recalled from an earlier version. Kimi Code carries this guidance in
+/// its tool descriptions and nowhere in its system prompt, so this profile does
+/// the same — the rule lands in the description of the tool being called.
+const EDIT_FILE_DESCRIPTION: &str = "Perform exact replacements in existing files.
+
+- Edit is mandatory for every incremental change, especially small edits. DO NOT use Write or \
+Bash `sed`.
+- Read the target file before every Edit. DO NOT call Edit from memory, stale context, or a \
+guessed `old_string`.
+- Take `old_string` and `new_string` from the Read output view, dropping the line-number prefix \
+and separator; match only file content.
+- `old_string` must be unique unless `replace_all` is set. If it is ambiguous, add surrounding \
+context. Use `replace_all` only when every occurrence should change — for example, renaming a \
+symbol throughout the file.
+- DO NOT issue consecutive Edit calls on the same file. A previous Edit can invalidate a later \
+Edit's `old_string`, causing `old_string not found`. Read the file again before the next Edit.
+- If an Edit fails with `old_string not found`, re-read the file and take the exact text from the \
+fresh output rather than guessing again.
+- Preserve existing indentation.";
 
 const GLOB_DESCRIPTION: &str = "Find files by search-root-relative path using a glob pattern. \
 Results are sorted lexicographically by relative path.
@@ -298,7 +306,7 @@ mod tests {
     }
 
     #[test]
-    fn edit_and_write_descriptions_drill_read_before_write() {
+    fn edit_and_write_descriptions_drill_reading_first() {
         let profile = KimiProfile::new("kimi-k3");
         let describe = |name: &str| {
             profile
@@ -310,14 +318,22 @@ mod tests {
                 .clone()
         };
 
+        // Kimi Code carries read-before-edit guidance in the tool descriptions
+        // and nowhere in its system prompt, so this is where it must land.
         for name in ["Edit", "Write"] {
             let text = describe(name);
             assert!(
-                text.contains("have not been read") || text.contains("has not been read"),
-                "{name} should warn about the read-before-write guard"
+                text.contains("Read"),
+                "{name} should steer the model to read the file first"
             );
         }
-        assert!(describe("Edit").contains("never reconstruct it from memory"));
+        assert!(
+            describe("Edit").contains("DO NOT call Edit from memory, stale context, or a guessed")
+        );
+        assert!(describe("Edit").contains("DO NOT issue consecutive Edit calls on the same file"));
+        assert!(describe("Write").contains("Read before overwriting an existing file"));
+        // Re-reading only to confirm a write landed is waste, not diligence.
+        assert!(describe("Read").contains("do not re-read solely to prove the write landed"));
 
         // Bash steers shell usage toward the dedicated tools, under the names
         // this profile actually exposes.
@@ -342,8 +358,8 @@ mod tests {
         // Fabro has no background shell; promising one would be a lie.
         assert!(!bash.contains("run_in_background"), "{bash}");
 
-        // Read explains that reading is what clears a file for writing.
-        assert!(describe("Read").contains("refuse a file that has not been read"));
+        // Read tells the model how to turn its output into an Edit old_string.
+        assert!(describe("Read").contains("Drop the number and separator"));
         // Grep must not promise ripgrep syntax: fabro falls back to POSIX grep.
         let grep = describe("Grep");
         assert!(grep.contains("POSIX"), "{grep}");
@@ -380,7 +396,10 @@ mod tests {
         let env = MockSandbox::linux();
         let prompt = profile.build_system_prompt(&env, &EnvContext::default(), &[], None, &[]);
         assert!(prompt.contains("You are Kimi"));
-        assert!(prompt.contains("# Reading Before Writing"));
+        assert!(prompt.contains("# Tracking Multi-Step Work"));
         assert!(prompt.contains("<environment>"));
+        // Kimi Code keeps read-before-edit mechanics out of its system prompt
+        // and in the tool descriptions; the profile follows that split.
+        assert!(!prompt.contains("Reading Before Writing"));
     }
 }

lib/components/fabro-agent/src/profiles/kimi_tools.rs

@@ -15,7 +15,7 @@
 //!
 //! Everything these tools do reaches the environment through the same
 //! [`Sandbox`](crate::sandbox::Sandbox) methods the built-ins use, so sandbox
-//! behavior, path policy, and the read-before-write guard are unchanged.
+//! behavior and path policy are unchanged.
 
 use std::collections::{HashMap, HashSet};
 use std::fmt::Write as _;
@@ -159,9 +159,6 @@ pub fn make_kimi_read_tool() -> RegisteredTool {
             NativeTool::ReadFile,
             "Read a text file from the workspace.
 
-Reading a file is also what clears it for writing: Edit and Write refuse a file that has not been \
-read in this session.
-
 - If you have a concrete path, call Read directly. Do not Glob or `ls` first to check that it \
 exists — a missing path returns an error you can handle.
 - When you need several files, emit multiple Read calls in one response rather than one per turn.
@@ -170,7 +167,9 @@ an Edit `old_string`.
 - `line_offset` is the 1-based first line to read. A NEGATIVE value reads from the end, so -100 \
 returns the last 100 lines.
 - `n_lines` defaults to 2000 lines.
-- Use Bash or an MCP tool for binary formats; this tool reads text.",
+- Use Bash or an MCP tool for binary formats; this tool reads text.
+- After a successful Edit or Write, do not re-read solely to prove the write landed. When the task \
+depends on an exact file, API, or output shape, inspect the final result before finishing.",
             serde_json::json!({
                 "type": "object",
                 "properties": {
@@ -238,7 +237,6 @@ returns the last 100 lines.
                 }
                 .map_err(|e| e.display_with_causes())?;
 
-                ctx.env.mark_agent_read(path);
                 Ok(content)
             })
         }),
@@ -260,16 +258,16 @@ pub fn make_kimi_write_tool() -> RegisteredTool {
     RegisteredTool {
         definition: definition(
             NativeTool::WriteFile,
-            "Create, append to, or replace a file.
-
-Read an existing file with Read before writing to it — this workspace refuses writes to files \
-that have not been read, and the call will fail.
+            "Create, append to, or replace a file entirely.
 
 - `mode` defaults to `overwrite`, which replaces the whole file. `append` requires an existing file \
 and adds to its end without inserting a newline.
-- Write is NOT for incremental changes to an existing file, however small. Use Edit instead: \
-overwrite replaces everything you did not restate.
-- Use `overwrite` when the file does not exist, or when you intend a complete replacement.
+- Write is NOT ALLOWED for incremental changes to existing files, including trivial, one-line, \
+quick, or cosmetic edits. Use Edit instead.
+- Use Write only when the file does not exist, you intend a complete replacement, or the new \
+contents have little continuity with the old contents.
+- Read before overwriting an existing file.
+- Write ignores the Read/Edit line-number view. NEVER include line prefixes.
 - Do not create documentation files that were not asked for.",
             serde_json::json!({
                 "type": "object",
@@ -328,7 +326,7 @@ overwrite replaces everything you did not restate.
 
 /// Kimi Code's `Edit` schema names the target `path`; fabro's shared edit
 /// executor calls it `file_path`. Translate only that adapter field and reuse
-/// the exact-match/read-before-write implementation.
+/// the exact-match implementation.
 #[must_use]
 pub fn make_kimi_edit_tool(description: &str) -> RegisteredTool {
     let shared = make_edit_file_tool();
@@ -513,7 +511,6 @@ mod tests {
     #[tokio::test]
     async fn edit_translates_kimi_path_to_the_shared_executor() {
         let env = sandbox_with("/f.txt", "before");
-        env.mark_agent_read("/f.txt");
         let tool = make_kimi_edit_tool("Edit");
 
         (tool.executor)(

lib/components/fabro-agent/src/profiles/mod.rs

@@ -536,11 +536,21 @@ mod tests {
                 );
             }
 
-            // The specific Kimi-only phrasing must not appear elsewhere.
-            assert!(
-                !anthropic_text.contains("has not been read"),
-                "read-before-write drilling leaked into {tool} for other profiles"
-            );
+            // The Kimi-only phrasing must not appear elsewhere. Assert it is
+            // present in Kimi's own description too: a one-sided check against
+            // a literal silently goes vacuous the next time that wording is
+            // rewritten, which is exactly how it last stopped testing anything.
+            if tool == NativeTool::EditFile {
+                const KIMI_EDIT_MARKER: &str = "DO NOT call Edit from memory";
+                assert!(
+                    kimi_text.contains(KIMI_EDIT_MARKER),
+                    "Kimi's {tool} description should drill reading before an edit: {kimi_text}"
+                );
+                assert!(
+                    !anthropic_text.contains(KIMI_EDIT_MARKER),
+                    "Kimi read-before-edit drilling leaked into {tool} for other profiles"
+                );
+            }
         }
     }
 

lib/components/fabro-agent/src/profiles/prompts/kimi.md.j2

@@ -24,16 +24,6 @@ Tool calls run behind the user's permission settings. A rejected or denied call
 
 When a tool call fails, diagnose why before acting again: read the error, check your assumptions, and make a focused adjustment. Do not retry the identical call blindly, but do not abandon a viable approach after a single failure either — if you are still stuck after investigating, ask the user.
 
-# Reading Before Writing
-
-This workspace refuses writes to files you have not read. `Edit` and `Write` both fail with "file exists but has not been read" when you target an existing file without reading it first. That failure costs a full turn and teaches you nothing you could not have known.
-
-- Call `Read` on the target before every `Edit` or `Write` against a file that already exists. No exceptions, including small or "obvious" edits.
-- Take `old_string` and `new_string` from what `Read` actually returned. Never construct `old_string` from memory, from an earlier version of the file, or from what you expect the file to contain.
-- If an `Edit` fails with "old_string not found", do not guess a different string. Re-read the file and take the exact text from the fresh output.
-- After you edit a file, its contents have changed. Re-read before your next edit to the same file rather than assuming your own edit landed as written.
-- `Write` fully replaces a file. Use it only for new files or a deliberate complete rewrite; for every incremental change use `Edit`.
-
 # Tracking Multi-Step Work
 
 Use `TodoList` for work that spans several steps, and keep it current as you go.

lib/components/fabro-agent/src/read_before_write_sandbox.rs

@@ -1 +0,0 @@
-pub use fabro_sandbox::read_guard::ReadBeforeWriteSandbox;

lib/components/fabro-agent/src/tool_execution.rs

@@ -570,13 +570,9 @@ mod tests {
         AgentQuestion, AgentQuestionAnswer, AgentQuestionAnswerStatus, AgentQuestionRuntime,
         AgentToolRuntime, register_question_tools,
     };
-    use crate::read_before_write_sandbox::ReadBeforeWriteSandbox;
-    use crate::test_support::{MockSandbox, MutableMockSandbox};
+    use crate::test_support::MockSandbox;
     use crate::tool_registry::{RegisteredTool, ToolContext, ToolRegistry, ToolSource};
-    use crate::tools::{
-        make_edit_file_tool, make_grep_tool, make_read_file_tool, make_shell_tool,
-        make_write_file_tool,
-    };
+    use crate::tools::make_shell_tool;
     use crate::types::SessionEvent;
 
     struct NamedPolicy {
@@ -1091,213 +1087,6 @@ mod tests {
         assert_eq!(*executions.lock().unwrap(), 0);
     }
 
-    // --- ReadBeforeWriteSandbox e2e tests ---
-
-    fn make_guarded_sandbox(files: HashMap<String, String>) -> Arc<dyn Sandbox> {
-        Arc::new(ReadBeforeWriteSandbox::new(Arc::new(
-            MutableMockSandbox::new(files),
-        )))
-    }
-
-    #[tokio::test]
-    async fn write_to_unread_file_blocked() {
-        let mut registry = ToolRegistry::new();
-        registry.register(make_write_file_tool());
-
-        let sandbox = make_guarded_sandbox(HashMap::from([("a.ts".into(), "content".into())]));
-        let tc = make_tool_call(
-            "write_file",
-            "call_1",
-            serde_json::json!({"file_path": "a.ts", "content": "new"}),
-        );
-        let emitter = Emitter::new();
-        let config = SessionOptions::default();
-
-        let result = execute_and_emit_one_tool(
-            &tc,
-            &registry,
-            sandbox,
-            None,
-            CancellationToken::new(),
-            &config,
-            &emitter,
-            "test-session",
-            "test-session",
-            None,
-        )
-        .await;
-
-        assert!(result.is_error);
-        assert!(result.content.to_string().contains("has not been read"));
-    }
-
-    #[tokio::test]
-    async fn read_then_write_succeeds() {
-        let mut registry = ToolRegistry::new();
-        registry.register(make_read_file_tool());
-        registry.register(make_write_file_tool());
-
-        let sandbox = make_guarded_sandbox(HashMap::from([("a.ts".into(), "content".into())]));
-        let emitter = Emitter::new();
-        let config = SessionOptions::default();
-
-        // First read the file
-        let read_tc = make_tool_call(
-            "read_file",
-            "call_1",
-            serde_json::json!({"file_path": "a.ts"}),
-        );
-        let read_result = execute_and_emit_one_tool(
-            &read_tc,
-            &registry,
-            sandbox.clone(),
-            None,
-            CancellationToken::new(),
-            &config,
-            &emitter,
-            "test-session",
-            "test-session",
-            None,
-        )
-        .await;
-        assert!(!read_result.is_error);
-
-        // Then write should succeed
-        let write_tc = make_tool_call(
-            "write_file",
-            "call_2",
-            serde_json::json!({"file_path": "a.ts", "content": "new"}),
-        );
-        let write_result = execute_and_emit_one_tool(
-            &write_tc,
-            &registry,
-            sandbox,
-            None,
-            CancellationToken::new(),
-            &config,
-            &emitter,
-            "test-session",
-            "test-session",
-            None,
-        )
-        .await;
-
-        assert!(!write_result.is_error);
-    }
-
-    #[tokio::test]
-    async fn grep_then_write_succeeds() {
-        let mut registry = ToolRegistry::new();
-        registry.register(make_grep_tool());
-        registry.register(make_write_file_tool());
-
-        let sandbox = make_guarded_sandbox(HashMap::from([("a.ts".into(), "content".into())]));
-        let emitter = Emitter::new();
-        let config = SessionOptions::default();
-
-        // Grep matching a.ts
-        let grep_tc = make_tool_call("grep", "call_1", serde_json::json!({"pattern": "content"}));
-        let grep_result = execute_and_emit_one_tool(
-            &grep_tc,
-            &registry,
-            sandbox.clone(),
-            None,
-            CancellationToken::new(),
-            &config,
-            &emitter,
-            "test-session",
-            "test-session",
-            None,
-        )
-        .await;
-        assert!(!grep_result.is_error);
-
-        // Then write should succeed
-        let write_tc = make_tool_call(
-            "write_file",
-            "call_2",
-            serde_json::json!({"file_path": "a.ts", "content": "new"}),
-        );
-        let write_result = execute_and_emit_one_tool(
-            &write_tc,
-            &registry,
-            sandbox,
-            None,
-            CancellationToken::new(),
-            &config,
-            &emitter,
-            "test-session",
-            "test-session",
-            None,
-        )
-        .await;
-
-        assert!(!write_result.is_error);
-    }
-
-    #[tokio::test]
-    async fn edit_unread_file_blocked() {
-        let mut registry = ToolRegistry::new();
-        registry.register(make_edit_file_tool());
-
-        let sandbox = make_guarded_sandbox(HashMap::from([("a.ts".into(), "content".into())]));
-        let tc = make_tool_call(
-            "edit_file",
-            "call_1",
-            serde_json::json!({"file_path": "a.ts", "old_string": "content", "new_string": "updated"}),
-        );
-        let emitter = Emitter::new();
-        let config = SessionOptions::default();
-
-        let result = execute_and_emit_one_tool(
-            &tc,
-            &registry,
-            sandbox,
-            None,
-            CancellationToken::new(),
-            &config,
-            &emitter,
-            "test-session",
-            "test-session",
-            None,
-        )
-        .await;
-
-        assert!(result.is_error);
-        assert!(result.content.to_string().contains("has not been read"));
-    }
-
-    #[tokio::test]
-    async fn write_new_file_succeeds() {
-        let mut registry = ToolRegistry::new();
-        registry.register(make_write_file_tool());
-
-        let sandbox = make_guarded_sandbox(HashMap::new());
-        let tc = make_tool_call(
-            "write_file",
-            "call_1",
-            serde_json::json!({"file_path": "new.ts", "content": "hello"}),
-        );
-        let emitter = Emitter::new();
-        let config = SessionOptions::default();
-
-        let result = execute_and_emit_one_tool(
-            &tc,
-            &registry,
-            sandbox,
-            None,
-            CancellationToken::new(),
-            &config,
-            &emitter,
-            "test-session",
-            "test-session",
-            None,
-        )
-        .await;
-
-        assert!(!result.is_error);
-    }
-
     fn shell_sandbox(result: fabro_sandbox::ExecResult) -> Arc<dyn Sandbox> {
         Arc::new(MockSandbox {
             exec_result: result,

lib/components/fabro-agent/src/tools.rs

@@ -138,7 +138,6 @@ pub fn make_read_file_tool() -> RegisteredTool {
                     .read_file(file_path, offset_usize, limit_usize)
                     .await
                     .map_err(|e| e.display_with_causes())?;
-                ctx.env.mark_agent_read(file_path);
                 Ok(content)
             })
         }),
@@ -441,27 +440,20 @@ pub fn make_grep_tool() -> RegisteredTool {
     }
 }
 
-/// Run a content search and mark every returned file as observed by the
-/// agent's read-before-write guard.
+/// Run a content search, rendering sandbox failures as tool-result strings.
+///
+/// Shared by the canonical `grep` tool and the Kimi profile's `Grep`, which
+/// group the same result lines differently.
 pub(crate) async fn execute_grep(
     ctx: &ToolContext,
     pattern: &str,
     path: &str,
     options: &GrepOptions,
 ) -> Result<Vec<String>, String> {
-    let results = ctx
-        .env
+    ctx.env
         .grep(pattern, path, options)
         .await
-        .map_err(|e| e.display_with_causes())?;
-    let mut seen_files = std::collections::HashSet::new();
-    for line in &results {
-        let file_path = grep_result_path(line, path);
-        if !file_path.is_empty() && seen_files.insert(file_path) {
-            ctx.env.mark_agent_read(file_path);
-        }
-    }
-    Ok(results)
+        .map_err(|e| e.display_with_causes())
 }
 
 /// Extract the file path from `<path>:<line>:<content>` grep output.
@@ -563,7 +555,6 @@ pub(crate) fn make_read_many_files_tool() -> RegisteredTool {
                 for (path, result) in results {
                     match result {
                         Ok(content) => {
-                            ctx.env.mark_agent_read(&path);
                             let _ = write!(output, "=== {path} ===\n{content}\n\n");
                         }
                         Err(err) => {
@@ -2286,69 +2277,4 @@ mod tests {
             "results should mention rust, got: {output}"
         );
     }
-
-    #[tokio::test]
-    async fn read_file_tool_marks_agent_read() {
-        use crate::read_before_write_sandbox::ReadBeforeWriteSandbox;
-
-        let mock = MockSandbox {
-            files: HashMap::from([("a.ts".into(), "content".into())]),
-            ..Default::default()
-        };
-        let env: Arc<dyn Sandbox> = Arc::new(ReadBeforeWriteSandbox::new(Arc::new(mock)));
-
-        // read_file tool should mark the file as agent-read
-        let tool = make_read_file_tool();
-        (tool.executor)(serde_json::json!({"file_path": "a.ts"}), ToolContext {
-            env:                 Arc::clone(&env),
-            cancel:              CancellationToken::new(),
-            tool_env_provider:   None,
-            session_id:          None,
-            root_session_id:     None,
-            tool_call_id:        None,
-            agent_event_emitter: None,
-        })
-        .await
-        .unwrap();
-
-        // write_file should succeed because read_file tool marked it
-        let result = env.write_file("a.ts", "new content").await;
-        assert!(
-            result.is_ok(),
-            "write should succeed after read_file tool marks agent-read"
-        );
-    }
-
-    #[tokio::test]
-    async fn grep_tool_marks_agent_read() {
-        use crate::read_before_write_sandbox::ReadBeforeWriteSandbox;
-
-        let mock = MockSandbox {
-            files: HashMap::from([("b.ts".into(), "content".into())]),
-            grep_results: vec!["b.ts:1:content".into()],
-            ..Default::default()
-        };
-        let env: Arc<dyn Sandbox> = Arc::new(ReadBeforeWriteSandbox::new(Arc::new(mock)));
-
-        // grep tool should mark matched files as agent-read
-        let tool = make_grep_tool();
-        (tool.executor)(serde_json::json!({"pattern": "content"}), ToolContext {
-            env:                 Arc::clone(&env),
-            cancel:              CancellationToken::new(),
-            tool_env_provider:   None,
-            session_id:          None,
-            root_session_id:     None,
-            tool_call_id:        None,
-            agent_event_emitter: None,
-        })
-        .await
-        .unwrap();
-
-        // write_file should succeed because grep tool marked it
-        let result = env.write_file("b.ts", "new content").await;
-        assert!(
-            result.is_ok(),
-            "write should succeed after grep tool marks agent-read"
-        );
-    }
 }

lib/components/fabro-sandbox/src/lib.rs

@@ -12,8 +12,6 @@ mod clone_source;
 #[cfg(any(feature = "docker", feature = "daytona", test))]
 mod managed_labels;
 
-pub mod read_guard;
-
 #[cfg(any(feature = "docker", feature = "daytona", test))]
 pub mod redact;
 
@@ -48,7 +46,6 @@ pub use provider::{
     LocalSandboxProvider, SandboxCreateSpec, SandboxLookupError, SandboxProvider,
     SandboxProviderRegistry,
 };
-pub use read_guard::ReadBeforeWriteSandbox;
 pub use reconnect::{reconnect, reconnect_for_run, reconnect_for_run_with_callback};
 pub use sandbox::{
     CommandOutputCallback, DEFAULT_EXEC_OUTPUT_TAIL_BYTES, DirEntry, ExecResult,

lib/components/fabro-sandbox/src/read_guard.rs

@@ -1,298 +0,0 @@
-use std::collections::HashSet;
-use std::path::{Component, PathBuf};
-use std::sync::{Arc, Mutex};
-
-use tracing::{debug, warn};
-
-use crate::Sandbox;
-
-/// Decorator that prevents writing to files the agent hasn't read first.
-///
-/// Tracks which file paths the agent has seen (via `mark_agent_read`, called by
-/// tool executors after agent-visible reads) and returns an error when
-/// `write_file` or `delete_file` targets an existing file that hasn't been
-/// read. Writing to new (non-existent) files is always allowed.
-pub struct ReadBeforeWriteSandbox {
-    inner:    Arc<dyn Sandbox>,
-    read_set: Mutex<HashSet<String>>,
-}
-
-impl ReadBeforeWriteSandbox {
-    pub fn new(inner: Arc<dyn Sandbox>) -> Self {
-        Self {
-            inner,
-            read_set: Mutex::new(HashSet::new()),
-        }
-    }
-
-    fn normalize_path(&self, path: &str) -> String {
-        let full = if path.starts_with('/') {
-            PathBuf::from(path)
-        } else {
-            PathBuf::from(self.inner.working_directory()).join(path)
-        };
-
-        let mut parts: Vec<String> = Vec::new();
-        for component in full.components() {
-            match component {
-                Component::Normal(s) => parts.push(s.to_string_lossy().into_owned()),
-                Component::ParentDir => {
-                    parts.pop();
-                }
-                Component::RootDir | Component::CurDir | Component::Prefix(_) => {}
-            }
-        }
-
-        format!("/{}", parts.join("/"))
-    }
-
-    fn mark_read(&self, path: &str) {
-        let normalized = self.normalize_path(path);
-        self.read_set
-            .lock()
-            .expect("read_set lock poisoned")
-            .insert(normalized);
-    }
-
-    fn has_read(&self, path: &str) -> bool {
-        let normalized = self.normalize_path(path);
-        self.read_set
-            .lock()
-            .expect("read_set lock poisoned")
-            .contains(&normalized)
-    }
-
-    async fn guard_write(&self, path: &str) -> crate::Result<()> {
-        let normalized = self.normalize_path(path);
-        if normalized.starts_with("/tmp/") {
-            return Ok(());
-        }
-        let exists = self.inner.file_exists(path).await?;
-        if exists && !self.has_read(path) {
-            warn!(path = %path, "Write blocked: file not read by agent");
-            Err(crate::Error::message(format!(
-                "Cannot write to '{path}': file exists but has not been read. \
-                 Use read_file to read the file before writing to it."
-            )))
-        } else {
-            Ok(())
-        }
-    }
-}
-
-crate::delegate_sandbox! {
-    ReadBeforeWriteSandbox => inner {
-        async fn write_file(&self, path: &str, content: &str) -> crate::Result<()> {
-            self.guard_write(path).await?;
-            self.inner.write_file(path, content).await
-        }
-
-        async fn delete_file(&self, path: &str) -> crate::Result<()> {
-            self.guard_write(path).await?;
-            self.inner.delete_file(path).await
-        }
-
-        fn mark_agent_read(&self, path: &str) {
-            debug!(path = %path, "File marked as agent-read");
-            self.mark_read(path);
-        }
-    }
-}
-
-#[cfg(test)]
-mod tests {
-    use std::collections::HashMap;
-
-    use super::*;
-    use crate::GrepOptions;
-    use crate::test_support::MockSandbox;
-
-    fn mock_with_files(files: HashMap<String, String>) -> MockSandbox {
-        MockSandbox {
-            files,
-            working_dir: "/work",
-            ..Default::default()
-        }
-    }
-
-    // Cycle 1: write to existing unread file → error
-    #[tokio::test]
-    async fn write_to_existing_unread_file_returns_error() {
-        let mock = mock_with_files(HashMap::from([("a.ts".into(), "content".into())]));
-        let env = ReadBeforeWriteSandbox::new(Arc::new(mock));
-
-        let result = env.write_file("a.ts", "new content").await;
-
-        assert!(result.is_err());
-        let err = result.unwrap_err().to_string();
-        assert!(err.contains("a.ts"));
-        assert!(err.contains("read"));
-    }
-
-    // Cycle 2: write to non-existent file → success
-    #[tokio::test]
-    async fn write_to_nonexistent_file_succeeds() {
-        let mock = mock_with_files(HashMap::new());
-        let env = ReadBeforeWriteSandbox::new(Arc::new(mock));
-
-        let result = env.write_file("new.ts", "content").await;
-
-        assert!(result.is_ok());
-    }
-
-    // Cycle 3: mark_agent_read then write → success
-    #[tokio::test]
-    async fn read_then_write_succeeds() {
-        let mock = mock_with_files(HashMap::from([("a.ts".into(), "content".into())]));
-        let env = ReadBeforeWriteSandbox::new(Arc::new(mock));
-
-        env.mark_agent_read("a.ts");
-        let result = env.write_file("a.ts", "new content").await;
-
-        assert!(result.is_ok());
-    }
-
-    // Cycle 4: read_file alone does NOT satisfy guard
-    #[tokio::test]
-    async fn read_file_alone_does_not_satisfy_guard() {
-        let mock = mock_with_files(HashMap::from([("a.ts".into(), "content".into())]));
-        let env = ReadBeforeWriteSandbox::new(Arc::new(mock));
-
-        env.read_file("a.ts", None, None).await.unwrap();
-        let result = env.write_file("a.ts", "new content").await;
-
-        assert!(result.is_err());
-    }
-
-    // Cycle 5: grep alone does NOT populate read set
-    #[tokio::test]
-    async fn grep_does_not_populate_read_set() {
-        let mock = MockSandbox {
-            files: HashMap::from([("b.ts".into(), "content".into())]),
-            grep_results: vec!["b.ts:1:content".into()],
-            working_dir: "/work",
-            ..Default::default()
-        };
-        let env = ReadBeforeWriteSandbox::new(Arc::new(mock));
-
-        env.grep("pattern", ".", &GrepOptions::default())
-            .await
-            .unwrap();
-        let result = env.write_file("b.ts", "new").await;
-
-        assert!(result.is_err());
-    }
-
-    // Cycle 6: mark_agent_read from grep results then write → success
-    #[tokio::test]
-    async fn mark_agent_read_from_grep_then_write_succeeds() {
-        let mock = MockSandbox {
-            files: HashMap::from([("b.ts".into(), "content".into())]),
-            grep_results: vec!["b.ts:1:content".into()],
-            working_dir: "/work",
-            ..Default::default()
-        };
-        let env = ReadBeforeWriteSandbox::new(Arc::new(mock));
-
-        env.mark_agent_read("b.ts");
-        let result = env.write_file("b.ts", "new").await;
-
-        assert!(result.is_ok());
-    }
-
-    // Cycle 7: glob does NOT populate read set
-    #[tokio::test]
-    async fn glob_does_not_populate_read_set() {
-        let mock = MockSandbox {
-            files: HashMap::from([("c.ts".into(), "content".into())]),
-            glob_results: vec!["c.ts".into()],
-            working_dir: "/work",
-            ..Default::default()
-        };
-        let env = ReadBeforeWriteSandbox::new(Arc::new(mock));
-
-        env.glob("*.ts", None).await.unwrap();
-        let result = env.write_file("c.ts", "new").await;
-
-        assert!(result.is_err());
-    }
-
-    // Cycle 8: path normalization — relative vs absolute via mark_agent_read
-    #[tokio::test]
-    async fn path_normalization_relative_and_absolute() {
-        let mock = MockSandbox {
-            files: HashMap::from([
-                ("a.ts".into(), "content".into()),
-                ("/work/a.ts".into(), "content".into()),
-            ]),
-            working_dir: "/work",
-            ..Default::default()
-        };
-        let env = ReadBeforeWriteSandbox::new(Arc::new(mock));
-
-        env.mark_agent_read("a.ts");
-        let result = env.write_file("/work/a.ts", "new content").await;
-
-        assert!(result.is_ok());
-    }
-
-    // Cycle 9: delete unread file → error
-    #[tokio::test]
-    async fn delete_unread_file_returns_error() {
-        let mock = mock_with_files(HashMap::from([("d.ts".into(), "content".into())]));
-        let env = ReadBeforeWriteSandbox::new(Arc::new(mock));
-
-        let result = env.delete_file("d.ts").await;
-
-        assert!(result.is_err());
-    }
-
-    // Cycle 10: error message is actionable
-    #[tokio::test]
-    async fn error_message_is_actionable() {
-        let mock = mock_with_files(HashMap::from([("main.rs".into(), "fn main() {}".into())]));
-        let env = ReadBeforeWriteSandbox::new(Arc::new(mock));
-
-        let err = env
-            .write_file("main.rs", "new")
-            .await
-            .unwrap_err()
-            .to_string();
-
-        assert!(err.contains("main.rs"));
-        assert!(err.contains("read_file"));
-    }
-
-    // Cycle 11: write to /tmp bypasses guard
-    #[tokio::test]
-    async fn write_to_tmp_bypasses_guard() {
-        let mock = MockSandbox {
-            files: HashMap::from([("/tmp/fabro-commit-msg".into(), "old".into())]),
-            working_dir: "/work",
-            ..Default::default()
-        };
-        let env = ReadBeforeWriteSandbox::new(Arc::new(mock));
-
-        let result = env.write_file("/tmp/fabro-commit-msg", "new").await;
-
-        assert!(result.is_ok());
-    }
-
-    #[tokio::test]
-    async fn stdio_process_forwards_to_inner_sandbox() {
-        let mock = Arc::new(MockSandbox::linux());
-        let env = ReadBeforeWriteSandbox::new(mock.clone());
-
-        env.spawn_stdio_process("python fake_agent.py", Some("/work/sub"), None, None)
-            .await
-            .unwrap();
-
-        assert_eq!(
-            *mock.captured_command.lock().unwrap(),
-            Some("python fake_agent.py".to_string())
-        );
-        assert_eq!(*mock.captured_working_dirs.lock().unwrap(), vec![Some(
-            "/work/sub".to_string()
-        )]);
-    }
-}

lib/components/fabro-sandbox/src/sandbox.rs

@@ -1211,12 +1211,6 @@ pub trait Sandbox: Send + Sync {
     ) -> crate::Result<Option<(String, HashMap<String, String>)>> {
         Ok(None)
     }
-
-    /// Record that the agent has explicitly read (seen) the given file path.
-    /// Called by tool executors after agent-visible reads (e.g. `read_file`,
-    /// `grep`). Default is a no-op; `ReadBeforeWriteSandbox` overrides to
-    /// populate its read set.
-    fn mark_agent_read(&self, _path: &str) {}
 }
 
 /// Resolve a path: relative paths are prepended with the working directory.

lib/components/fabro-workflow/src/pipeline/initialize.rs

@@ -12,8 +12,7 @@ use fabro_graphviz::graph;
 use fabro_hooks::{HookContext, HookDecision, HookEvent, HookExecutionContext, HookRunner};
 use fabro_model::Catalog;
 use fabro_sandbox::{
-    GitSetupIntent, ReadBeforeWriteSandbox, SandboxEventCallback, SandboxSpec,
-    reconnect_for_run_with_callback, shell_quote,
+    GitSetupIntent, SandboxEventCallback, SandboxSpec, reconnect_for_run_with_callback, shell_quote,
 };
 use fabro_static::EnvVars;
 use fabro_vault::Vault;
@@ -374,15 +373,13 @@ pub async fn initialize(
         .await
         .map_err(|err| Error::engine_with_anyhow("Failed to reconnect sandbox for resume", err))?;
         sandbox_initialized = false;
-        Arc::new(ReadBeforeWriteSandbox::new(Arc::from(sandbox)))
+        Arc::from(sandbox)
     } else {
-        Arc::new(ReadBeforeWriteSandbox::new(
-            options
-                .sandbox
-                .build(Some(Arc::clone(&sandbox_event_callback)))
-                .await
-                .map_err(|e| Error::engine_with_anyhow("Failed to build sandbox", e))?,
-        ))
+        options
+            .sandbox
+            .build(Some(Arc::clone(&sandbox_event_callback)))
+            .await
+            .map_err(|e| Error::engine_with_anyhow("Failed to build sandbox", e))?
     };
     let cleanup_guard = (!attach_existing).then(|| {
         scopeguard::guard(Arc::clone(&sandbox), |sandbox| {