todo: extract storage interface with in-memory implementation

[rumpl]

Introduce a TodoStorage interface to decouple the todo toolset from its storage layer.

Changes

• TodoStorage interface — defines the storage contract with 6 methods: Add, All, Len, FindByID, Update, and Clear.
• MemoryTodoStorage — in-memory, concurrency-safe implementation backed by concurrent.Slice[Todo].
• WithStorage functional option — lets callers inject their own storage implementation into NewTodoTool.
• NewTodoTool now accepts variadic TodoOptions. Defaults to MemoryTodoStorage when no option is provided, so all existing call sites remain unchanged.
• Tests updated to exercise the new storage option, including a dedicated TestTodoTool_WithStorage test.

Assisted-By: cagent

[docker-agent]

Review Summary

Assessment: 🟡 NEEDS ATTENTION

This PR introduces a clean abstraction with the TodoStorage interface, successfully decoupling the todo toolset from its storage implementation. However, there are 3 concurrency and safety issues in the newly added code that should be addressed:

1. Nil storage validation — WithStorage can accept nil, causing panics
2. ID generation race conditions — Both createTodo and createTodos have non-atomic read-modify-write sequences that can produce duplicate IDs in concurrent scenarios

These issues are all in the changed code and should be fixed before merging.

---

Findings: 3 issues

[docker-agent]

MEDIUM: WithStorage option can set nil storage causing nil pointer dereference

The WithStorage function accepts a TodoStorage parameter and assigns it directly to t.handler.storage without nil validation. If called as WithStorage(nil), subsequent method calls like h.storage.Len() or h.storage.Add() will panic with nil pointer dereferences.

Recommendation: Add validation:

func WithStorage(storage TodoStorage) TodoOption {
    return func(t *TodoTool) {
        if storage == nil {
            panic("storage cannot be nil")
        }
        t.handler.storage = storage
    }
}

Or document that nil is invalid and will cause panics.

[rumpl]

/review

[docker-agent]

Review Summary

Assessment: 🟢 APPROVE

This PR successfully introduces a clean storage abstraction for the todo toolset. The implementation is well-designed:

✅ Interface Design: TodoStorage provides a clear contract with 6 well-defined methods
✅ Thread Safety: MemoryTodoStorage properly uses concurrent.Slice for concurrency-safe operations
✅ Backward Compatibility: Default storage behavior preserved via functional options pattern
✅ ID Generation: Atomic counter ensures globally unique IDs across concurrent operations
✅ Test Coverage: Comprehensive tests including storage injection and nil-safety

The code was reviewed for potential race conditions and concurrency issues. All operations are properly protected by the underlying concurrent data structures. The atomic ID counter ensures unique IDs even under concurrent load, and the storage abstraction allows for future implementations (e.g., persistent storage) without changing the tool interface.

No blocking issues found. The changed code works correctly as designed.