diff --git a/src/ClawdDotNet.App/AppHost.cs b/src/ClawdDotNet.App/AppHost.cs index 61a3c80..ce4de41 100644 --- a/src/ClawdDotNet.App/AppHost.cs +++ b/src/ClawdDotNet.App/AppHost.cs @@ -761,7 +761,14 @@ public sealed class AppHost : IAsyncDisposable // #716: Late work waits for headroom the rest of the day does not need, and a // day the budget will not last is named while there is still time to act. lateRunGate: headroom, - budgetOutlook: headroom); + budgetOutlook: headroom, + // #714: A task tree that has come to rest is reviewed by the lead of its unit. + treeWatch: new TaskTreeWatch( + taskRepository, + (task, ct) => board.CreateAsync(task, ct), + stateStore, + () => engine.OrganisationProvider?.Invoke(), + loggerFactory.CreateLogger("ClawdDotNet.Core.Tasks.TreeWatch"))); // Reconciliation nicht blockierend: Der Start soll nicht auf das Dateisystem warten. _ = Task.Run(async () => diff --git a/src/ClawdDotNet.Core/Tasks/TaskScanner.cs b/src/ClawdDotNet.Core/Tasks/TaskScanner.cs index f6f349a..86d80a6 100644 --- a/src/ClawdDotNet.Core/Tasks/TaskScanner.cs +++ b/src/ClawdDotNet.Core/Tasks/TaskScanner.cs @@ -137,6 +137,7 @@ public sealed class TaskScanner : IAsyncDisposable private readonly ILateRunGate? _lateRunGate; private readonly IBudgetOutlook? _outlook; + private readonly ITaskTreeWatch? _treeWatch; /// How often the day ahead is looked at — a forecast, not a watchdog. public static readonly TimeSpan OutlookInterval = TimeSpan.FromMinutes(30); @@ -163,8 +164,10 @@ public sealed class TaskScanner : IAsyncDisposable int maxParallelRuns = DefaultMaxParallelRuns, Func? budgetMissPolicy = null, ILateRunGate? lateRunGate = null, - IBudgetOutlook? budgetOutlook = null) + IBudgetOutlook? budgetOutlook = null, + ITaskTreeWatch? treeWatch = null) { + _treeWatch = treeWatch; _repo = repo; _dispatcher = dispatcher; _market = market ?? new AlwaysOpenMarketCalendar(); @@ -286,6 +289,10 @@ public sealed class TaskScanner : IAsyncDisposable SummarizeMissesIfNewDay(now); await WarnAboutShortfallsAsync(candidates, now, ct); + // #714: resting task trees are looked at — the watch decides itself how often. + if (_treeWatch is not null) + await _treeWatch.CheckAsync(now, ct); + var due = new List<(TaskItem Task, string Occurrence, bool Late)>(); foreach (var task in candidates) { diff --git a/src/ClawdDotNet.Core/Tasks/TaskTreeWatch.cs b/src/ClawdDotNet.Core/Tasks/TaskTreeWatch.cs new file mode 100644 index 0000000..aba44cd --- /dev/null +++ b/src/ClawdDotNet.Core/Tasks/TaskTreeWatch.cs @@ -0,0 +1,211 @@ +using System.Security.Cryptography; +using System.Text; +using ClawdDotNet.Core.Organisation; +using ClawdDotNet.Core.State; +using Microsoft.Extensions.Logging; + +namespace ClawdDotNet.Core.Tasks; + +/// A check the scanner runs on every tick; it decides itself how often it acts. +public interface ITaskTreeWatch +{ + Task CheckAsync(DateTime nowUtc, CancellationToken ct); +} + +/// An open task whose whole subtree has come to rest. +public sealed record RestingTree(TaskItem Root, IReadOnlyList Below); + +/// +/// #714: A task tree that has stopped moving is looked at. Taken from Paperclip's "task +/// watchdog" (Organisation-Konzept, 9a): agents stop for wrong reasons — "done" without +/// evidence, a review nobody does, a blocker read wrongly, an error given up on that could +/// have been fixed. Without a check the tree simply lies still, and nobody notices. +/// +/// Detecting is code, interpreting is a model. Whether a tree rests is a query +/// over the board (this class, the first piece of #646). Whether the rest is justified is +/// decided by the lead of the unit the tree belongs to — or a human, where there is no lead +/// — through a review task. The reviewer may send work back (comment, reopen), not do it +/// itself (the line of #680). +/// +/// Once per state of rest. The review is not repeated while the tree stays +/// exactly as it was: a reviewer who accepts the rest is not asked again. Only when the +/// tree has moved and comes to rest in a different state is it looked at again. The review +/// task is not part of the tree, so it does not change what it reviews. +/// +public sealed class TaskTreeWatch( + ITaskRepository repo, + Func createTask, + IStateStore state, + Func organisation, + ILogger logger) : ITaskTreeWatch +{ + /// How often the board is looked at. + public static readonly TimeSpan Interval = TimeSpan.FromMinutes(30); + + private const string StateKeyPrefix = "tasktree.rest."; + + private DateTime _lastCheckAt = DateTime.MinValue; + + public async Task CheckAsync(DateTime nowUtc, CancellationToken ct) + { + if (nowUtc - _lastCheckAt < Interval) + return; + _lastCheckAt = nowUtc; + + try + { + var all = await repo.ListAllAsync(includeArchived: false, ct); + foreach (var tree in FindResting(all)) + await ReviewOnceAsync(tree, ct); + } + catch (Exception ex) when (ex is not OperationCanceledException) + { + logger.LogWarning(ex, "Check of resting task trees failed — next attempt in {Minutes} min", Interval.TotalMinutes); + } + } + + /// + /// The open tasks whose subtree has come to rest — only the topmost of them, so a tree is + /// reviewed once and not again for each of its resting branches. Pure, so the rules can + /// be tested without an instance. + /// + public static IReadOnlyList FindResting(IReadOnlyList all) + { + var byId = all.ToDictionary(t => t.Id, StringComparer.OrdinalIgnoreCase); + var children = all + .Where(t => !string.IsNullOrWhiteSpace(t.ParentId) && byId.ContainsKey(t.ParentId)) + .GroupBy(t => t.ParentId, StringComparer.OrdinalIgnoreCase) + .ToDictionary(g => g.Key, g => g.ToList(), StringComparer.OrdinalIgnoreCase); + + List Below(string id) + { + var result = new List(); + var seen = new HashSet(StringComparer.OrdinalIgnoreCase) { id }; + var pending = new Stack(); + pending.Push(id); + while (pending.TryPop(out var current)) + { + foreach (var child in children.GetValueOrDefault(current) ?? []) + { + if (!seen.Add(child.Id)) + continue; // a cycle in the data must not hang this + result.Add(child); + pending.Push(child.Id); + } + } + return result; + } + + var resting = new Dictionary(StringComparer.OrdinalIgnoreCase); + foreach (var root in all.Where(t => children.ContainsKey(t.Id) && IsOpen(t))) + { + var below = Below(root.Id); + if (below.Count > 0 && below.All(IsAtRest) && IsAtRest(root)) + resting[root.Id] = new RestingTree(root, below); + } + + // Only the topmost: a resting tree inside another resting tree is part of that one. + return resting.Values + .Where(tree => !Ancestors(tree.Root, byId).Any(a => resting.ContainsKey(a.Id))) + .OrderBy(tree => tree.Root.Id, StringComparer.OrdinalIgnoreCase) + .ToList(); + } + + /// Not finished: the tree still has a purpose to reach. + private static bool IsOpen(TaskItem task) + => task.Status is not (TaskItemStatus.Done or TaskItemStatus.Canceled or TaskItemStatus.Archived); + + /// + /// Nothing will move this task by itself: it is not running, and the scanner will not + /// start it — a todo for an agent is still on its way, a todo for a human is not. + /// + private static bool IsAtRest(TaskItem task) => task.Status switch + { + TaskItemStatus.InProgress => false, + TaskItemStatus.Todo => TaskAssignee.KindOf(task.Assignee) == TaskAssigneeKind.Human, + _ => true + }; + + private static IEnumerable Ancestors(TaskItem task, IReadOnlyDictionary byId) + { + var seen = new HashSet(StringComparer.OrdinalIgnoreCase) { task.Id }; + var next = task.ParentId; + while (!string.IsNullOrWhiteSpace(next) && seen.Add(next) && byId.TryGetValue(next, out var parent)) + { + yield return parent; + next = parent.ParentId; + } + } + + /// What the tree looks like — the same value means nothing moved since the last review. + public static string Fingerprint(RestingTree tree) + { + var text = string.Join("|", new[] { tree.Root }.Concat(tree.Below) + .OrderBy(t => t.Id, StringComparer.OrdinalIgnoreCase) + .Select(t => $"{t.Id}:{TaskText.Of(t.Status)}")); + return Convert.ToHexString(SHA256.HashData(Encoding.UTF8.GetBytes(text)))[..16]; + } + + private async Task ReviewOnceAsync(RestingTree tree, CancellationToken ct) + { + var key = StateKeyPrefix + tree.Root.Id; + var fingerprint = Fingerprint(tree); + if (await state.GetAsync(key, ct) == fingerprint) + return; + + var reviewer = ReviewerOf(tree.Root, organisation()); + await createTask(BuildReview(tree, reviewer), ct); + await state.SetAsync(key, fingerprint, ct); + + logger.LogInformation( + "Task tree {Root} has come to rest ({Count} tasks below) — review assigned to {Reviewer}", + tree.Root.Id, tree.Below.Count, reviewer); + } + + /// + /// The lead of the unit whose board the tree is on — or, where that unit has none, the + /// lead above it; without any, a human. + /// + public static string ReviewerOf(TaskItem root, OrgChart? org) + { + if (org is not { HasUnits: true } || TaskBoard.UnitOf(root, org) is not { } unitId) + return TaskAssignee.Human; + + foreach (var unit in new[] { org.Unit(unitId)! }.Concat(org.Ancestors(unitId))) + { + if (org.LeaderOf(unit.Id) is { } lead) + return "@" + lead.AgentId; + } + return TaskAssignee.Human; + } + + /// The review task. It stands beside the tree, not in it — it must not change what it reviews. + public static TaskItem BuildReview(RestingTree tree, string reviewer) + { + var body = new StringBuilder(); + body.AppendLine($"The work under {tree.Root.Id} \"{tree.Root.Title}\" has come to rest: nothing in it is running or about to run, but {tree.Root.Id} is not finished."); + body.AppendLine(); + body.AppendLine($"- {tree.Root.Id} [{TaskText.Of(tree.Root.Status)}] {tree.Root.Title} ({tree.Root.Assignee})"); + foreach (var task in tree.Below.OrderBy(t => t.Id, StringComparer.OrdinalIgnoreCase)) + body.AppendLine($" - {task.Id} [{TaskText.Of(task.Status)}] {task.Title} ({task.Assignee}{(string.IsNullOrWhiteSpace(task.ParentId) ? "" : $", serves {task.ParentId}")})"); + body.AppendLine(); + body.AppendLine("Decide whether the standstill is justified. Read the tasks and their comments. Typical wrong stops:"); + body.AppendLine("- marked done without evidence that the result exists,"); + body.AppendLine("- waiting for a review that nobody does,"); + body.AppendLine("- blocked by something that is already finished, or was misread,"); + body.AppendLine("- an error given up on that could have been fixed."); + body.AppendLine(); + body.AppendLine("If it is justified, say why in your result. If not, send the work back: comment on the task and reopen it (task_comment with reopen), or finish or cancel the top task. Do not do the work yourself — your job is to judge it, not to redo it."); + + return new TaskItem + { + Title = $"Review: work under {tree.Root.Id} has come to rest", + Body = body.ToString().TrimEnd(), + Assignee = reviewer, + Priority = Math.Clamp(tree.Root.Priority, 1, 5), + Status = TaskItemStatus.Todo, + Type = TaskItemType.Work, + Acceptance = "The result says, for each wrongly stopped task, what was sent back — or why the standstill is justified." + }; + } +} diff --git a/tests/ClawdDotNet.Core.Tests/Tasks/TaskTreeWatchTests.cs b/tests/ClawdDotNet.Core.Tests/Tasks/TaskTreeWatchTests.cs new file mode 100644 index 0000000..f78cf53 --- /dev/null +++ b/tests/ClawdDotNet.Core.Tests/Tasks/TaskTreeWatchTests.cs @@ -0,0 +1,209 @@ +using ClawdDotNet.Core.Config; +using ClawdDotNet.Core.Organisation; +using ClawdDotNet.Core.State; +using ClawdDotNet.Core.Storage; +using ClawdDotNet.Core.Tasks; +using Microsoft.Extensions.Logging.Abstractions; +using Shouldly; + +namespace ClawdDotNet.Core.Tests.Tasks; + +/// +/// #714: A task tree that has stopped moving while its purpose is not reached gets a review +/// — once per state of rest, by the lead of its unit. +/// +public sealed class TaskTreeWatchTests +{ + private static TaskItem T(string id, TaskItemStatus status, string parent = "", string assignee = "@ads") + => new() { Id = id, Title = id, Status = status, ParentId = parent, Assignee = assignee }; + + // ═══════════════════ what rests ═══════════════════ + + [Fact] + public void An_open_task_whose_sub_tasks_all_stopped_rests() + { + var trees = TaskTreeWatch.FindResting( + [ + T("goal", TaskItemStatus.Blocked), + T("a", TaskItemStatus.Done, "goal"), + T("b", TaskItemStatus.Blocked, "goal"), + T("c", TaskItemStatus.InReview, "goal") + ]); + + trees.ShouldHaveSingleItem().Root.Id.ShouldBe("goal"); + trees[0].Below.Count.ShouldBe(3); + } + + [Theory] + [InlineData(TaskItemStatus.InProgress)] + [InlineData(TaskItemStatus.Todo)] + public void A_tree_with_anything_running_or_about_to_run_does_not_rest(TaskItemStatus moving) + => TaskTreeWatch.FindResting( + [ + T("goal", TaskItemStatus.Blocked), + T("a", TaskItemStatus.Done, "goal"), + T("b", moving, "goal") + ]).ShouldBeEmpty(); + + [Fact] + public void A_task_waiting_for_a_human_does_not_move_by_itself() + => TaskTreeWatch.FindResting( + [ + T("goal", TaskItemStatus.Todo, assignee: "@human"), + T("a", TaskItemStatus.Done, "goal") + ]).ShouldHaveSingleItem(); + + [Fact] + public void A_finished_tree_is_not_a_standstill() + => TaskTreeWatch.FindResting( + [ + T("goal", TaskItemStatus.Done), + T("a", TaskItemStatus.Done, "goal") + ]).ShouldBeEmpty(); + + [Fact] + public void A_single_task_without_sub_tasks_is_not_a_tree() + => TaskTreeWatch.FindResting([T("alone", TaskItemStatus.Blocked)]).ShouldBeEmpty(); + + [Fact] + public void Only_the_topmost_resting_tree_is_reported() + { + var trees = TaskTreeWatch.FindResting( + [ + T("goal", TaskItemStatus.Blocked), + T("part", TaskItemStatus.Blocked, "goal"), + T("leaf", TaskItemStatus.Done, "part") + ]); + + trees.ShouldHaveSingleItem().Root.Id.ShouldBe("goal"); + trees[0].Below.Select(t => t.Id).ShouldBe(["part", "leaf"], ignoreOrder: true); + } + + [Fact] + public void A_cycle_in_the_data_does_not_hang_the_search() + => Should.NotThrow(() => TaskTreeWatch.FindResting( + [ + T("a", TaskItemStatus.Blocked, "b"), + T("b", TaskItemStatus.Blocked, "a") + ])); + + // ═══════════════════ who reviews ═══════════════════ + + private static OrgChart Shop(bool amazonHasLead = true) => new( + [ + new UnitConfig { Id = "marketing", Name = "Marketing", LeaderAgentId = "marketing-lead" }, + new UnitConfig { Id = "amazon", Name = "Amazon", ParentId = "marketing", LeaderAgentId = amazonHasLead ? "amazon-lead" : null } + ], + [ + new AgentConfig { AgentId = "marketing-lead", UnitId = "marketing" }, + new AgentConfig { AgentId = "amazon-lead", UnitId = "amazon" }, + new AgentConfig { AgentId = "ads", UnitId = "amazon" } + ]); + + [Fact] + public void The_lead_of_the_tree_s_unit_reviews_or_the_lead_above() + { + TaskTreeWatch.ReviewerOf(T("goal", TaskItemStatus.Blocked), Shop()).ShouldBe("@amazon-lead"); + TaskTreeWatch.ReviewerOf(T("goal", TaskItemStatus.Blocked), Shop(amazonHasLead: false)).ShouldBe("@marketing-lead"); + } + + [Fact] + public void Without_units_or_a_lead_a_human_reviews() + { + TaskTreeWatch.ReviewerOf(T("goal", TaskItemStatus.Blocked), null).ShouldBe("@human"); + TaskTreeWatch.ReviewerOf(T("goal", TaskItemStatus.Blocked, assignee: "@human"), Shop()).ShouldBe("@human"); + } + + [Fact] + public void The_review_stands_beside_the_tree_and_may_only_send_work_back() + { + var tree = new RestingTree(T("goal", TaskItemStatus.Blocked), [T("a", TaskItemStatus.Done, "goal")]); + + var review = TaskTreeWatch.BuildReview(tree, "@amazon-lead"); + + review.ParentId.ShouldBeEmpty("inside the tree it would change what it reviews"); + review.Assignee.ShouldBe("@amazon-lead"); + review.Body.ShouldContain("a [done]"); + review.Body.ShouldContain("Do not do the work yourself"); + } + + // ═══════════════════ once per state of rest ═══════════════════ + + private sealed class MemoryState : IStateStore + { + private readonly Dictionary _values = new(); + public Task GetAsync(string key, CancellationToken ct) => Task.FromResult(_values.GetValueOrDefault(key)); + public Task SetAsync(string key, string value, CancellationToken ct) { _values[key] = value; return Task.CompletedTask; } + public Task DeleteAsync(string key, CancellationToken ct) { _values.Remove(key); return Task.CompletedTask; } + } + + private static async Task<(SqliteTaskRepository Repo, string Directory)> BoardWith(params TaskItem[] tasks) + { + var directory = Path.Combine(Path.GetTempPath(), "clawd-tests", "watch-" + Guid.NewGuid().ToString("N")[..8]); + var repo = new SqliteTaskRepository(new SqliteStorage(Path.Combine(directory, "state.db"))); + foreach (var task in tasks) + { + await repo.UpsertAsync(task, default); + await repo.SetStatusAsync(task.Id, task.Status, DateTime.UtcNow, default); + } + return (repo, directory); + } + + private static void CleanUp(string directory) + { + SqliteStorage.ReleasePooledConnections(directory); + try { Directory.Delete(directory, recursive: true); } + catch { /* cleaning up is secondary */ } + } + + [Fact] + public async Task A_rest_is_reviewed_once_and_again_only_after_the_tree_moved() + { + var (repo, directory) = await BoardWith(T("goal", TaskItemStatus.Blocked), T("a", TaskItemStatus.Done, "goal")); + try + { + var created = new List(); + var watch = new TaskTreeWatch(repo, (t, _) => { created.Add(t); return Task.CompletedTask; }, + new MemoryState(), () => null, NullLogger.Instance); + var now = new DateTime(2027, 3, 10, 12, 0, 0, DateTimeKind.Utc); + + await watch.CheckAsync(now, default); + created.ShouldHaveSingleItem(); + + await watch.CheckAsync(now + TaskTreeWatch.Interval, default); + created.Count.ShouldBe(1, "nothing moved — a reviewer who accepted the rest is not asked again"); + + // Reopened, worked on, stuck again: a different state of rest. + await repo.SetStatusAsync("a", TaskItemStatus.Blocked, DateTime.UtcNow, default); + await watch.CheckAsync(now + 2 * TaskTreeWatch.Interval, default); + created.Count.ShouldBe(2); + } + finally + { + CleanUp(directory); + } + } + + [Fact] + public async Task The_board_is_looked_at_only_every_interval() + { + var (repo, directory) = await BoardWith(T("goal", TaskItemStatus.Blocked), T("a", TaskItemStatus.Done, "goal")); + try + { + var created = new List(); + var watch = new TaskTreeWatch(repo, (t, _) => { created.Add(t); return Task.CompletedTask; }, + new MemoryState(), () => null, NullLogger.Instance); + var now = new DateTime(2027, 3, 10, 12, 0, 0, DateTimeKind.Utc); + + await watch.CheckAsync(now, default); + await repo.SetStatusAsync("a", TaskItemStatus.Canceled, DateTime.UtcNow, default); + await watch.CheckAsync(now + TimeSpan.FromMinutes(1), default); + + created.Count.ShouldBe(1, "the next look is due after the interval, not on every tick"); + } + finally + { + CleanUp(directory); + } + } +}