Skip to content

Commit f8e83c8

Browse files
yl-jiangJiangYulin
andauthored
fix(tools): update todo items in place instead of duplicating (#135)
* fix(tools): todo_write updates existing items in-place instead of duplicating When an agent marks a todo item done by calling todo_write with checked=True, the tool previously always appended a new [x] line, leaving the original [ ] line intact. This produced duplicate entries. The tool now performs an upsert: - If the item exists as [ ], replace it with [x] in-place. - If it is already in the target state, return a no-op result. - Otherwise, append as before. Adds test_todo_write_upsert to cover the new behaviour. Co-authored-by: Copilot * fix(tools): simplify todo_write tool description for LLM clarity Co-authored-by: Copilot --------- Co-authored-by: JiangYulin <yulin@JiangYulindeMacBook-Pro.local>
1 parent e537ba9 commit f8e83c8

3 files changed

Lines changed: 43 additions & 4 deletions

File tree

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ The format is based on Keep a Changelog, and this project currently tracks chang
2222

2323
### Fixed
2424

25+
- `todo_write` tool now updates an existing unchecked item in-place when `checked=True` instead of appending a duplicate `[x]` line.
26+
2527
- React TUI spinner now stays visible throughout the entire agent turn: `assistant_complete` no longer resets `busy` state prematurely, and `tool_started` explicitly sets `busy=true` so the status bar remains active even when tool calls follow an assistant message. `line_complete` is the sole signal that ends the turn and clears the spinner.
2628
- Skill loader now uses `yaml.safe_load` to parse SKILL.md frontmatter, correctly handling YAML block scalars (`>`, `|`), quoted values, and other standard YAML constructs instead of naive line-by-line splitting.
2729
- `BackendHostConfig` was missing the `cwd` field, causing `AttributeError: 'BackendHostConfig' object has no attribute 'cwd'` on startup when `oh` was run after the runtime refactor that added `cwd` support to `build_runtime`.

src/openharness/tools/todo_write_tool.py

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,16 +18,29 @@ class TodoWriteToolInput(BaseModel):
1818

1919

2020
class TodoWriteTool(BaseTool):
21-
"""Append an item to a TODO markdown file."""
21+
"""Add or update an item in a TODO markdown file."""
2222

2323
name = "todo_write"
24-
description = "Append a TODO item to a markdown checklist file."
24+
description = "Add a new TODO item or mark an existing one as done in a markdown checklist file."
2525
input_model = TodoWriteToolInput
2626

2727
async def execute(self, arguments: TodoWriteToolInput, context: ToolExecutionContext) -> ToolResult:
2828
path = Path(context.cwd) / arguments.path
29-
prefix = "- [x]" if arguments.checked else "- [ ]"
3029
existing = path.read_text(encoding="utf-8") if path.exists() else "# TODO\n"
31-
updated = existing.rstrip() + f"\n{prefix} {arguments.item}\n"
30+
31+
unchecked_line = f"- [ ] {arguments.item}"
32+
checked_line = f"- [x] {arguments.item}"
33+
target_line = checked_line if arguments.checked else unchecked_line
34+
35+
if unchecked_line in existing and arguments.checked:
36+
# Mark existing unchecked item as done (in-place update)
37+
updated = existing.replace(unchecked_line, checked_line, 1)
38+
elif target_line in existing:
39+
# Item already in desired state — no-op
40+
return ToolResult(output=f"No change needed in {path}")
41+
else:
42+
# New item — append
43+
updated = existing.rstrip() + f"\n{target_line}\n"
44+
3245
path.write_text(updated, encoding="utf-8")
3346
return ToolResult(output=f"Updated {path}")

tests/test_tools/test_core_tools.py

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,30 @@ async def test_skill_todo_and_config_tools(tmp_path: Path, monkeypatch):
135135
assert config_result.output == "Updated theme"
136136

137137

138+
@pytest.mark.asyncio
139+
async def test_todo_write_upsert(tmp_path: Path):
140+
tool = TodoWriteTool()
141+
ctx = ToolExecutionContext(cwd=tmp_path)
142+
143+
await tool.execute(TodoWriteToolInput(item="task A"), ctx)
144+
await tool.execute(TodoWriteToolInput(item="task B"), ctx)
145+
146+
# Marking done should update in-place, not append a duplicate
147+
result = await tool.execute(TodoWriteToolInput(item="task A", checked=True), ctx)
148+
assert result.is_error is False
149+
150+
content = (tmp_path / "TODO.md").read_text(encoding="utf-8")
151+
assert content.count("task A") == 1
152+
assert "- [x] task A" in content
153+
assert "- [ ] task A" not in content
154+
assert "- [ ] task B" in content
155+
156+
# Calling again with same state is a no-op
157+
noop = await tool.execute(TodoWriteToolInput(item="task A", checked=True), ctx)
158+
assert "No change" in noop.output
159+
assert (tmp_path / "TODO.md").read_text(encoding="utf-8").count("task A") == 1
160+
161+
138162
@pytest.mark.asyncio
139163
async def test_notebook_edit_tool(tmp_path: Path):
140164
result = await NotebookEditTool().execute(

0 commit comments

Comments
 (0)