From db0d11ea2e3ab4d1a62892c2254c98d5896ca8d5 Mon Sep 17 00:00:00 2001 From: Felix Martin Date: Mon, 1 Jun 2026 18:01:28 -0400 Subject: [PATCH] Add runFetchAsync helper and migrate the tasks fetch Co-Authored-By: Claude Opus 4.8 (1M context) --- internal/session/roles.go | 65 +++++++++++++++++++++++++++------------ 1 file changed, 45 insertions(+), 20 deletions(-) diff --git a/internal/session/roles.go b/internal/session/roles.go index d1a8436..8e588fb 100644 --- a/internal/session/roles.go +++ b/internal/session/roles.go @@ -81,6 +81,37 @@ func (c *Controller) SetTasks(p tasks.Provider) { c.mu.Unlock() } +// runFetchAsync launches a generation-guarded background fetch. The caller has +// already captured its dependencies and (for the *Locked callers) holds c.mu; +// this method only spawns the goroutine, which re-acquires the lock itself. +// +// Closure contract: +// - fetch performs the I/O and runs with NO lock held; it must not touch c +// fields without taking c.mu itself. +// - stale runs under the re-acquired c.mu and returns true to DISCARD the +// result (e.g. a newer generation superseded this one). +// - apply runs under the re-acquired c.mu and records the result (persisting +// itself when the role requires it); it must NOT call c.notify — the helper +// owns the post-unlock notify, and notify self-locks, so calling it under +// c.mu would deadlock. +// +// On a non-stale completion the helper notifies after releasing the lock. +func (c *Controller) runFetchAsync(timeout time.Duration, fetch func(ctx context.Context), stale func() bool, apply func()) { + go func() { + ctx, cancel := context.WithTimeout(context.Background(), timeout) + defer cancel() + fetch(ctx) + c.mu.Lock() + if stale() { + c.mu.Unlock() + return + } + apply() + c.mu.Unlock() + c.notify() + }() +} + // startTasksFetchLocked kicks off an asynchronous Today() fetch when a provider // is set. Mirrors RequestCoach: generation-guarded, discards stale or // post-planning results, and notifies on completion. Caller holds mu. @@ -94,26 +125,20 @@ func (c *Controller) startTasksFetchLocked() { gen := c.tasksGen c.tasksStatus = tasksPending provider := c.tasksProvider - go func() { - ctx, cancel := context.WithTimeout(context.Background(), tasksTimeout) - defer cancel() - list, err := provider.Today(ctx) - - c.mu.Lock() - if gen != c.tasksGen || c.runtimeState != domain.RuntimePlanning { - c.mu.Unlock() - return // stale or left planning: discard - } - if err != nil { - c.tasksStatus = tasksError - c.tasksList = nil - } else { - c.tasksStatus = tasksReady - c.tasksList = list - } - c.mu.Unlock() - c.notify() - }() + var list []tasks.Task + var err error + c.runFetchAsync(tasksTimeout, + func(ctx context.Context) { list, err = provider.Today(ctx) }, + func() bool { return gen != c.tasksGen || c.runtimeState != domain.RuntimePlanning }, + func() { + if err != nil { + c.tasksStatus = tasksError + c.tasksList = nil + } else { + c.tasksStatus = tasksReady + c.tasksList = list + } + }) } // SetKnowledge injects the Knowledge source. Mirrors SetTasks. A nil source