diff --git a/agent/moon.pkg b/agent/moon.pkg index 436c03e11..c89d8187d 100644 --- a/agent/moon.pkg +++ b/agent/moon.pkg @@ -4,6 +4,7 @@ import { "bobzhang/openseek/agent_tool", "bobzhang/openseek/agent_tool/edit", "bobzhang/openseek/agent_tool/finish", + "bobzhang/openseek/agent_tool/read_state", "bobzhang/openseek/agent_tool/moon_check", "bobzhang/openseek/agent_tool/read", "bobzhang/openseek/agent_tool/shell", diff --git a/agent/tool_definition.mbt b/agent/tool_definition.mbt index 646cb3fb0..9f7dc5334 100644 --- a/agent/tool_definition.mbt +++ b/agent/tool_definition.mbt @@ -12,11 +12,15 @@ fn[X] tool_definitions( // `moon check` through shell). ignore(scope) let workspace_root = runtime.workspace_root() + // One seen-set shared by the file tools for this session: `read` and `edit` + // record what the agent has looked at, and `write` uses it to flag an + // overwrite of a file that was never read this session. + let read_state = @read_state.ReadState::ReadState() Tools([ @shell.definition(workspace_root~), - @read.definition(workspace_root~), + @read.definition(workspace_root~, read_state~), @edit.definition(workspace_root~), - @write.definition(workspace_root~), + @write.definition(workspace_root~, read_state~), @finish.definition(), ]) } diff --git a/agent_tool/read/moon.pkg b/agent_tool/read/moon.pkg index f80427c83..4c8d2df2d 100644 --- a/agent_tool/read/moon.pkg +++ b/agent_tool/read/moon.pkg @@ -1,6 +1,7 @@ import { "bobzhang/openseek/agent_tool", "bobzhang/openseek/agent_tool/internal/error" @tool_error, + "bobzhang/openseek/agent_tool/read_state", "bobzhang/openseek/agent_tool/read/internal/decode", "bobzhang/openseek/internal/workspace_path", "bobzhang/openseek/testkit/filesystem" @vfs, diff --git a/agent_tool/read/pkg.generated.mbti b/agent_tool/read/pkg.generated.mbti index 1193bcacc..e13df9d1d 100644 --- a/agent_tool/read/pkg.generated.mbti +++ b/agent_tool/read/pkg.generated.mbti @@ -3,10 +3,11 @@ package "bobzhang/openseek/agent_tool/read" import { "bobzhang/openseek/agent_tool", + "bobzhang/openseek/agent_tool/read_state", } // Values -pub fn definition(workspace_root? : String) -> @agent_tool.AgentToolDefinition +pub fn definition(workspace_root? : String, read_state? : @read_state.ReadState) -> @agent_tool.AgentToolDefinition // Errors diff --git a/agent_tool/read/read.mbt b/agent_tool/read/read.mbt index f7aa0433f..96c18af12 100644 --- a/agent_tool/read/read.mbt +++ b/agent_tool/read/read.mbt @@ -32,6 +32,7 @@ /// invalid arguments. pub fn definition( workspace_root? : String = ".", + read_state? : @read_state.ReadState, ) -> @agent_tool.AgentToolDefinition { AgentToolDefinition( name="read", @@ -49,18 +50,21 @@ pub fn definition( "max_output_chars": { "type": "number" }, }, }), - execute=Async(arguments => execute_with_workspace(workspace_root, arguments)), + execute=Async(arguments => { + execute_with_workspace(workspace_root, read_state, arguments) + }), ) } ///| async fn execute(arguments : Json) -> @agent_tool.ToolAction { - execute_with_workspace(".", arguments) + execute_with_workspace(".", None, arguments) } ///| async fn execute_with_workspace( workspace_root : String, + read_state : @read_state.ReadState?, arguments : Json, ) -> @agent_tool.ToolAction { let input = @decode.decode(arguments) catch { @@ -72,7 +76,11 @@ async fn execute_with_workspace( ) } } - read_file(input, path=@workspace_path.resolve(workspace_root, input.path)) + read_file( + input, + path=@workspace_path.resolve(workspace_root, input.path), + read_state~, + ) } ///| @@ -90,10 +98,20 @@ priv struct RenderedSelection { truncated : Bool } +///| +/// Record `path` as seen this session, when read-state tracking is wired in. +fn record_seen(read_state : @read_state.ReadState?, path : String) -> Unit { + match read_state { + Some(state) => state.record(path) + None => () + } +} + ///| async fn read_file( input : @decode.ReadInput, path~ : String, + read_state~ : @read_state.ReadState?, ) -> @agent_tool.ToolAction { let brief = "read \{@agent_tool.brief_basename(path)}" match directory_read_error(path) { @@ -114,6 +132,9 @@ async fn read_file( // containing only a newline is not empty (it has blank lines) and renders // normally. if content == "" { + // An empty file has no content the agent could miss, so it counts as fully + // seen for the purpose of the unread-overwrite check. + record_seen(read_state, path) return @agent_tool.ToolAction::respond( "start_line=\{input.start_line} shown_lines=0 total_lines=0 truncated=false note=empty file", is_error=false, @@ -122,6 +143,15 @@ async fn read_file( } let selection = select_lines(content, input.start_line, input.max_lines) let rendered = render_numbered_content(selection, input.max_output_chars) + // Record the file as seen only when this read showed the agent the whole file: + // every line was selected (`shown_lines == total_lines`, so the range started + // at the top and was not cut short — a high `max_lines` that still covers the + // file counts) and the output budget did not truncate the body. A sliced or + // truncated read leaves content unseen, so a later `write` must still be + // flagged as an unread overwrite. + if selection.shown_lines == selection.total_lines && !rendered.truncated { + record_seen(read_state, path) + } // Every read has the same shape: numbered lines followed by a single status // footer. When the body is empty (start past EOF, or a zero budget) only the // footer is returned, so the model still learns what it asked for. @@ -582,6 +612,36 @@ async test "read rejects non-object arguments" { assert_true(output.is_error) } +///| +async test "read marks a file seen only on a complete read" { + @vfs.with_tmpdir(dir => { + let path = "\{dir}/notes.txt" + @fs.write_file(path, "a\nb\nc\nd\n", create_mode=CreateOrTruncate) + // A sliced read (max_lines) leaves most of the file unseen, so it must NOT + // be recorded — a later overwrite should still be flagged. + let sliced = @read_state.ReadState::ReadState() + guard definition(read_state=sliced).execute is Async(read_sliced) + let _ = read_sliced({ "path": path, "max_lines": 1 }) + assert_false(sliced.has_seen(path)) + // A read from a later line is likewise partial. + let offset = @read_state.ReadState::ReadState() + guard definition(read_state=offset).execute is Async(read_offset) + let _ = read_offset({ "path": path, "start_line": 2 }) + assert_false(offset.has_seen(path)) + // A complete read (default args, untruncated) records the file as seen. + let full = @read_state.ReadState::ReadState() + guard definition(read_state=full).execute is Async(read_full) + let _ = read_full({ "path": path }) + assert_true(full.has_seen(path)) + // A bounded read whose max_lines comfortably covers the file is still a full + // view, so it also counts as seen. + let bounded = @read_state.ReadState::ReadState() + guard definition(read_state=bounded).execute is Async(read_bounded) + let _ = read_bounded({ "path": path, "max_lines": 1000 }) + assert_true(bounded.has_seen(path)) + }) +} + ///| async test "a huge max_lines returns the remaining lines without overflow" { @vfs.with_tmpdir(dir => { diff --git a/agent_tool/read_state/moon.pkg b/agent_tool/read_state/moon.pkg new file mode 100644 index 000000000..a6a50eada --- /dev/null +++ b/agent_tool/read_state/moon.pkg @@ -0,0 +1 @@ +warnings = "+unnecessary_annotation" diff --git a/agent_tool/read_state/pkg.generated.mbti b/agent_tool/read_state/pkg.generated.mbti new file mode 100644 index 000000000..fd54cb04f --- /dev/null +++ b/agent_tool/read_state/pkg.generated.mbti @@ -0,0 +1,19 @@ +// Generated using `moon info`, DON'T EDIT IT +package "bobzhang/openseek/agent_tool/read_state" + +// Values + +// Errors + +// Types and methods +pub struct ReadState { + seen : Map[String, Unit] +} +pub fn ReadState::ReadState() -> Self +pub fn ReadState::has_seen(Self, String) -> Bool +pub fn ReadState::record(Self, String) -> Unit + +// Type aliases + +// Traits + diff --git a/agent_tool/read_state/read_state.mbt b/agent_tool/read_state/read_state.mbt new file mode 100644 index 000000000..5b3c4c533 --- /dev/null +++ b/agent_tool/read_state/read_state.mbt @@ -0,0 +1,39 @@ +///| +/// Session-scoped record of which workspace files the agent has already seen +/// this run — read, written, or edited. `write` consults it to flag when it +/// overwrites a file the agent never read, so a wholesale replacement of unseen +/// content is at least visible in the transcript. +/// +/// It is ephemeral runtime state shared across the file tools, never persisted: +/// a resumed session starts empty, which fails safe (the agent is nudged to +/// read before overwriting). Keys are the already-resolved paths the tools +/// operate on — each tool resolves `arguments.path` under the workspace root +/// before recording or checking — so the same file maps to the same key across +/// tools. +/// +/// This deliberately tracks only "seen vs not seen", not modification times: +/// the "file changed since you read it" check is for editors/linters touching +/// files concurrently, which does not happen in a single batch CLI session. +pub struct ReadState { + seen : Map[String, Unit] +} + +///| +/// A fresh, empty seen-set. The agent loop creates one per session and hands it +/// to the read/write/edit tools. +pub fn ReadState::ReadState() -> ReadState { + { seen: {} } +} + +///| +/// Record that the agent has seen `path` (call after a successful read, write, +/// or edit). Idempotent. +pub fn ReadState::record(self : ReadState, path : String) -> Unit { + self.seen[path] = () +} + +///| +/// Whether `path` has been read, written, or edited this session. +pub fn ReadState::has_seen(self : ReadState, path : String) -> Bool { + self.seen.get(path) is Some(_) +} diff --git a/agent_tool/read_state/read_state_test.mbt b/agent_tool/read_state/read_state_test.mbt new file mode 100644 index 000000000..ff70731c8 --- /dev/null +++ b/agent_tool/read_state/read_state_test.mbt @@ -0,0 +1,11 @@ +///| +test "ReadState records and recalls seen paths" { + let state = @read_state.ReadState::ReadState() + assert_false(state.has_seen("/ws/parser.mbt")) + state.record("/ws/parser.mbt") + assert_true(state.has_seen("/ws/parser.mbt")) + // Unrelated paths stay unseen; recording is idempotent. + assert_false(state.has_seen("/ws/types.mbt")) + state.record("/ws/parser.mbt") + assert_true(state.has_seen("/ws/parser.mbt")) +} diff --git a/agent_tool/write/moon.pkg b/agent_tool/write/moon.pkg index 74fd7f247..9a3b079c3 100644 --- a/agent_tool/write/moon.pkg +++ b/agent_tool/write/moon.pkg @@ -2,6 +2,7 @@ import { "bobzhang/openseek/agent_tool", "bobzhang/openseek/agent_tool/internal/auto_check", "bobzhang/openseek/agent_tool/internal/error" @tool_error, + "bobzhang/openseek/agent_tool/read_state", "bobzhang/openseek/agent_tool/write/internal/decode", "bobzhang/openseek/internal/workspace_path", "bobzhang/openseek/testkit/filesystem" @vfs, diff --git a/agent_tool/write/pkg.generated.mbti b/agent_tool/write/pkg.generated.mbti index ab2081075..d705c618f 100644 --- a/agent_tool/write/pkg.generated.mbti +++ b/agent_tool/write/pkg.generated.mbti @@ -3,10 +3,11 @@ package "bobzhang/openseek/agent_tool/write" import { "bobzhang/openseek/agent_tool", + "bobzhang/openseek/agent_tool/read_state", } // Values -pub fn definition(workspace_root? : String) -> @agent_tool.AgentToolDefinition +pub fn definition(workspace_root? : String, read_state? : @read_state.ReadState) -> @agent_tool.AgentToolDefinition // Errors diff --git a/agent_tool/write/write.mbt b/agent_tool/write/write.mbt index 9abf7d641..fd1e99d0a 100644 --- a/agent_tool/write/write.mbt +++ b/agent_tool/write/write.mbt @@ -18,6 +18,7 @@ /// invalid arguments. pub fn definition( workspace_root? : String = ".", + read_state? : @read_state.ReadState, ) -> @agent_tool.AgentToolDefinition { AgentToolDefinition( name="write", @@ -30,18 +31,21 @@ pub fn definition( }, "required": ["path", "content"], }), - execute=Async(arguments => execute_with_workspace(workspace_root, arguments)), + execute=Async(arguments => { + execute_with_workspace(workspace_root, read_state, arguments) + }), ) } ///| async fn execute(arguments : Json) -> @agent_tool.ToolAction { - execute_with_workspace(".", arguments) + execute_with_workspace(".", None, arguments) } ///| async fn execute_with_workspace( workspace_root : String, + read_state : @read_state.ReadState?, arguments : Json, ) -> @agent_tool.ToolAction { let input = @decode.decode(arguments) catch { @@ -54,13 +58,14 @@ async fn execute_with_workspace( } } let path = @workspace_path.resolve(workspace_root, input.path) - write_file(input, path~) + write_file(input, path~, read_state~) } ///| async fn write_file( input : @decode.WriteInput, path~ : String, + read_state~ : @read_state.ReadState?, ) -> @agent_tool.ToolAction { try { match manifest_write_error(path, input.content) { @@ -78,18 +83,39 @@ async fn write_file( // never read is a common way to lose work. A non-ENOENT `exists` error (e.g. // ENOTDIR) falls through to the write below and the `catch` reports it. let existed = @fs.exists(path) + // When a session-scoped `ReadState` is wired in (by the agent loop), call + // out an overwrite of a file the agent never read this session — the case + // most likely to lose work it could not see. Without one (standalone use) + // we keep the plain overwrite note. + let unread_overwrite = existed && + (match read_state { + Some(state) => !state.has_seen(path) + None => false + }) @fs.write_file(path, input.content, create_mode=CreateOrTruncate) + // A successful write means the agent has now seen this file's content. + match read_state { + Some(state) => state.record(path) + None => () + } let chars = input.content.length() - let summary = if existed { - "ok: wrote \{chars} chars to \{path} (overwrote existing file)" + let note = if unread_overwrite { + " (overwrote existing file you had not read this session)" + } else if existed { + " (overwrote existing file)" } else { - "ok: wrote \{chars} chars to \{path}" + "" } - let content = @auto_check.append_summary(path, summary) + let content = @auto_check.append_summary( + path, + "ok: wrote \{chars} chars to \{path}\{note}", + ) @agent_tool.ToolAction::respond( content, brief=@agent_tool.brief_line( - if existed { + if unread_overwrite { + "overwrote unread \{input.path}" + } else if existed { "overwrote \{input.path}" } else { "wrote \{chars} chars to \{input.path}" @@ -312,6 +338,45 @@ async test "write reports whether it created or overwrote a file" { }) } +///| +async test "write flags overwriting a file the agent never read this session" { + @vfs.with_tmpdir(dir => { + let path = "\{dir}/existing.txt" + @fs.write_file(path, "original content", create_mode=CreateOrTruncate) + let state = @read_state.ReadState::ReadState() + guard definition(read_state=state).execute is Async(write_with_state) + // Overwriting a pre-existing file the agent never read is called out. + guard write_with_state({ "path": path, "content": "new" }) is Respond(first) + assert_true( + first.content.has_suffix( + "(overwrote existing file you had not read this session)", + ), + ) + // That write recorded the file as seen, so a second overwrite is the plain + // note — no repeated "had not read" warning. + guard write_with_state({ "path": path, "content": "newer" }) + is Respond(second) + assert_true(second.content.has_suffix("(overwrote existing file)")) + assert_false(second.content.contains("had not read")) + }) +} + +///| +async test "write does not flag a file already recorded as seen" { + @vfs.with_tmpdir(dir => { + let path = "\{dir}/seen.txt" + @fs.write_file(path, "original", create_mode=CreateOrTruncate) + let state = @read_state.ReadState::ReadState() + // Simulate the read tool having recorded this path earlier this session. + state.record(path) + guard definition(read_state=state).execute is Async(write_with_state) + guard write_with_state({ "path": path, "content": "updated" }) + is Respond(output) + assert_true(output.content.has_suffix("(overwrote existing file)")) + assert_false(output.content.contains("had not read")) + }) +} + ///| async test "write accepts empty content" { @vfs.with_tmpdir(dir => {