Key todo provider session locks by identity, not service ID

[PratikDhanave]

Summary

The todo context provider keys its per-session lock registry by Session.ServiceID(). This is the same latent issue that was corrected for the agentmode provider in #491, and it breaks the synchronization guarantee the per-session lock exists to provide:

• a service ID may be empty, collapsing unrelated sessions onto a single shared "_default" lock;
• distinct sessions can share a service ID and unexpectedly share a lock;
• the service ID is mutable, so one session can resolve to different locks over its lifetime;
• registry entries were never removed and accumulated for the provider's lifetime.

Fix

Key the registry by session object identity via weak.Pointer[agent.Session], with a runtime.AddCleanup that drops each entry once its session is garbage-collected (the cleanup captures only the weak key, never the session). A nil/absent session uses the shared fallback lock.

This mirrors the approach taken for agentmode in #491 and — notably — restores parity with .NET: the existing code comment already stated the intent was to match ".NET's per-session SemaphoreSlim via ConditionalWeakTable," but ConditionalWeakTable is identity-keyed, so the ServiceID-keyed Go implementation had actually diverged from it. Identity keying is the alignment.

Public API

No exported symbols change; the fix is internal to the todo provider's lock registry.

Tests

Adds getsessionlock_internal_test.go (package todo) covering the same cases requested during the #491 review:

• the same *agent.Session always receives the same lock;
• distinct sessions with empty service IDs receive distinct locks;
• distinct sessions with the same service ID receive distinct locks;
• a nil/absent session uses the shared fallback lock;
• stale registry entries do not accumulate (drained after GC via the weak-key cleanup).

go test -race -shuffle=on ./agent/harness/todo passes; go build ./... and go vet ./... clean; gofumpt clean.

Requires Go 1.25 (weak, runtime.AddCleanup), which the module already targets.

[Copilot]

Pull request overview

This pull request fixes the todo context provider’s per-session lock registry so it is keyed by session object identity rather than Session.ServiceID(), avoiding lock collisions/mutation issues and preventing unbounded registry growth by removing entries after session GC.

Changes:

• Reworked Provider.getSessionLock to key the lock registry by weak.Pointer[agent.Session] and to register a runtime.AddCleanup callback that deletes the entry when the session is collected.
• Updated provider documentation/comments to accurately describe the identity-keyed behavior and the GC cleanup behavior.
• Added internal tests validating lock identity semantics and that stale registry entries are cleaned up after GC.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
agent/harness/todo/todo.go	Keys session locks by weak session identity and deletes lock entries via runtime.AddCleanup on session GC.
agent/harness/todo/getsessionlock_internal_test.go	Adds coverage for same-session lock stability, distinct-session lock separation, nil-session fallback, and cleanup of stale entries.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[qmuntal]

Move these tests to todo_test.go

[PratikDhanave]

Done — moved into todo_test.go as a black-box concurrency test (concurrent tool invocations + public reads on a shared session). I dropped the white-box getSessionLock identity tests: as with the agentmode fix you merged (#491), lock identity is an implementation detail with no clean black-box observation (distinct vs. shared locks are both race-free). Happy to restore explicit white-box identity coverage if you'd prefer.