Implement undo_edit tool for reverting file edits

t-242·WorkTask····Omni/Agent/Tools.hs

Created3 months ago·Updated1 month ago

Description

Add an undo_edit tool that reverts the last edit made to a file.

Design Overview

This requires tracking edit history. The simplest approach is to store the previous content of each file before editing it.

Data Types

data UndoEditArgs = UndoEditArgs
  { undoPath :: Text  -- Path to file to undo
  }
  deriving (Show, Eq, Generic)

Implementation

Step 1: Add Edit History Tracking

Modify executeEditFile to save the original content before making changes. Use an IORef (Map FilePath Text) to store the previous content.

The edit history needs to be passed through the engine. Look at how Engine.hs passes configuration to tools. You may need to: 1. Add an engineEditHistory :: IORef (Map FilePath Text) field to EngineConfig 2. Pass this to tool execution 3. Have executeEditFile update the map before editing

Step 2: Create the Undo Tool

undoEditTool :: IORef (Map FilePath Text) -> Engine.Tool
undoEditTool historyRef = Engine.Tool
  { Engine.toolName = "undo_edit",
    Engine.toolDescription = "Undo the last edit made to a file. Only works for files edited in this session.",
    Engine.toolJsonSchema = ...,
    Engine.toolExecute = executeUndoEdit historyRef
  }

Step 3: Implement executeUndoEdit

1. Look up the path in the history map 2. If found, write the previous content back to the file 3. Remove the entry from the map (so you cannot undo twice) 4. Return success/failure

Tool Schema

{
  "type": "object",
  "properties": {
    "path": { "type": "string", "description": "Absolute path to the file to undo" }
  },
  "required": ["path"]
}

Testing

1. Create a temp file with known content 2. Edit it with edit_file 3. Call undo_edit 4. Verify the file has the original content

Notes

Only track ONE level of undo per file (keeps it simple)
The history map should be created fresh for each agent run in Worker.hs
If a file is edited multiple times, only the most recent edit can be undone
Add undoEditTool to the exports and allTools list

Timeline (4)

🔄[human]Open → InProgress1 month ago

💬[human]1 month ago

CHANGES REQUESTED: ## Code Review

Task Completion

The implementation accomplishes what the task requested:

✅ Added UndoEditArgs data type
✅ Added EditHistory type (IORef (Map FilePath Text))
✅ Created editFileToolWithHistory that saves original content before editing
✅ Created undoEditTool that reverts files using the history
✅ Implemented executeUndoEdit that writes previous content back and removes entry from map
✅ Added allToolsWithHistory function
✅ Added tests for the new functionality
✅ Updated exports

Build/Test Status

✅ Build passes
✅ Tests pass

Lint Issues

There are lint warnings: 1. File length: 1019 lines (max 1000) - This is a warning, not a blocker 2. Missing shebang - Minor warning 3. hlint suggestion: Use newtype instead of data for UndoEditArgs - This is a valid suggestion since it's a single-field record

Code Quality Observations

1. Design is reasonable: The approach of having both allTools (with a stub undoEditToolStub) and allToolsWithHistory is a sensible way to maintain backward compatibility while enabling the new functionality.

2. The stub returns a helpful error message: Good UX for anyone using allTools instead of allToolsWithHistory.

3. History properly removed after undo: Prevents double-undo as specified.

4. Minor issue: The test imports Data.IORef directly (Data.IORef.newIORef) instead of using the already-imported qualified newIORef from the import list at line 48. This is inconsistent but not a bug.

Missing Integration

The task description mentions: > "The history map should be created fresh for each agent run in Worker.hs"

Looking at the diff, there's no change to Worker.hs to actually use allToolsWithHistory. The tools are implemented but not integrated into the actual agent runtime. This may be intentional (split into separate tasks) but worth noting.

Summary

The implementation is solid and follows the task specification. The lint issues are minor (file length warning and hlint suggestion to use newtype). The code builds and tests pass. The lack of Worker.hs integration means the feature isn't fully wired up, but the Tools.hs changes themselves are complete and correct.

The hlint suggestion to use newtype instead of data for UndoEditArgs should be applied since it's a single-field record and this is flagged as a lint failure (fail: lint: hlint).

🔄[human]InProgress → Done1 month ago