From 451c01848c7fab267c7afd1c2e14a1808d94b381 Mon Sep 17 00:00:00 2001 From: mika kuns Date: Thu, 10 Sep 2026 13:14:39 +0200 Subject: [PATCH] fix(worker): start_list_handler ehrlich machen, wenn keine App verbunden ist ConnectedClientTracker zaehlt verbundene UI-Clients (WorkerHub.OnConnectedAsync/ OnDisconnectedAsync). StartListHandler prueft ihn vor dem Broadcast und liefert bei 0 verbundenen Clients Requested=false plus Reason statt unbedingtem true. Tool-Beschreibung und external-mcp.md an das reale Verhalten angepasst. --- docs/explore-notes/external-mcp.md | 8 ++ src/ClaudeDo.Worker/CLAUDE.md | 2 +- .../External/ListHandlerMcpTools.cs | 23 +++- .../Hub/ConnectedClientTracker.cs | 19 +++ src/ClaudeDo.Worker/Hub/WorkerHub.cs | 19 ++- src/ClaudeDo.Worker/Program.cs | 1 + .../External/ListHandlerMcpToolsTests.cs | 113 ++++++++++++++++++ 7 files changed, 177 insertions(+), 8 deletions(-) create mode 100644 src/ClaudeDo.Worker/Hub/ConnectedClientTracker.cs create mode 100644 tests/ClaudeDo.Worker.Tests/External/ListHandlerMcpToolsTests.cs diff --git a/docs/explore-notes/external-mcp.md b/docs/explore-notes/external-mcp.md index 19eed615..6ab450ba 100644 --- a/docs/explore-notes/external-mcp.md +++ b/docs/explore-notes/external-mcp.md @@ -98,6 +98,7 @@ Daily prep: `GetDailyPrepCandidates`, `SetMyDay`. | `TaskWaitMcpTools` | `WaitForTaskChange` | | `QueueStateMcpTools` | `GetQueueState` | | `AttachmentMcpTools` | `AddTaskAttachment`, `ListTaskAttachments`, `RemoveTaskAttachment` | +| `ListHandlerMcpTools` | `StartListHandler` | ## Per-tool behaviour worth knowing @@ -235,6 +236,13 @@ task already outside those two (unknown ids reported as status `"NotFound"`, als **`AttachmentMcpTools`** — re-attaching the same `fileName` overwrites. Add/remove refuse on a `Running` task. +**`StartListHandler`** — the worker only broadcasts `ListHandlerStartRequested`; the connected +ClaudeDo App does the actual work (creates the handler task, opens the triage ConPTY tile). Before +broadcasting it checks `ConnectedClientTracker` (a `WorkerHub.OnConnectedAsync`/`OnDisconnectedAsync`-driven +singleton count) — with zero connected clients the broadcast would reach nobody, so the tool +returns `Requested: false` plus a `Reason` instead of claiming success. Does not queue or retry the +request for a later connect. + **`GetDailyPrepCandidates`** — Idle, non-blocked tasks in a git repo **not** excluded by `AppSettings.ReportExcludedPaths` and not already `IsMyDay`, plus the current Idle MyDay tasks and `maxTasks` (= `DailyPrepMaxTasks`). Repo-exclusion logic lives in the `DailyPrepFilter` diff --git a/src/ClaudeDo.Worker/CLAUDE.md b/src/ClaudeDo.Worker/CLAUDE.md index 0d1dbf40..d3c332c4 100644 --- a/src/ClaudeDo.Worker/CLAUDE.md +++ b/src/ClaudeDo.Worker/CLAUDE.md @@ -30,7 +30,7 @@ Worker/ Findings/ — FindingsStore + FindingsStoreLocator: the per-project `.claudedo/` trap store Git/ — GitExcludeWriter (`.git/info/exclude`) + GitHead, shared by skill seeding and findings Config/ — WorkerConfig - Hub/ — WorkerHub, HubBroadcaster + Hub/ — WorkerHub, HubBroadcaster, ConnectedClientTracker (connected-UI-client count) Logging/ — LogRingBuffer (30-min window) + BroadcastLogSink (Serilog → footer + overlay) Report/ — ClaudeHistoryReader, WeekReportPromptBuilder, WeekReportService Prime/ — "Prime Claude" schedules: PrimeScheduler, PrimeRunner, PrimePrompts, diff --git a/src/ClaudeDo.Worker/External/ListHandlerMcpTools.cs b/src/ClaudeDo.Worker/External/ListHandlerMcpTools.cs index 6c8d873f..5712d99b 100644 --- a/src/ClaudeDo.Worker/External/ListHandlerMcpTools.cs +++ b/src/ClaudeDo.Worker/External/ListHandlerMcpTools.cs @@ -7,32 +7,36 @@ using TaskStatus = ClaudeDo.Data.Models.TaskStatus; namespace ClaudeDo.Worker.External; -public sealed record StartListHandlerResult(bool Requested, string ListId, string ListName, int TaskCount, IReadOnlyList TaskNumbers); +public sealed record StartListHandlerResult(bool Requested, string ListId, string ListName, int TaskCount, IReadOnlyList TaskNumbers, string? Reason = null); // The "Let Claude handle it" button, reachable from outside the UI. The worker only broadcasts; // the App owns the ConPTY tile and creates the handler task exactly as the button does // (MissionControlViewModel.OpenMergeHelperConPtySessionAsync), so there is one code path for -// starting a run, not two. Without a connected App nothing happens -- the result says so. +// starting a run, not two. Without a connected App the broadcast has nobody to receive it, so +// ConnectedClientTracker is checked first and Requested comes back false with a Reason instead of +// an unconditional true. [McpServerToolType] public sealed class ListHandlerMcpTools { private readonly ListRepository _lists; private readonly TaskRepository _tasks; private readonly HubBroadcaster _broadcaster; + private readonly ConnectedClientTracker _clients; - public ListHandlerMcpTools(ListRepository lists, TaskRepository tasks, HubBroadcaster broadcaster) + public ListHandlerMcpTools(ListRepository lists, TaskRepository tasks, HubBroadcaster broadcaster, ConnectedClientTracker clients) { _lists = lists; _tasks = tasks; _broadcaster = broadcaster; + _clients = clients; } [McpServerTool, Description( "Start a list-handler run (\"Let Claude handle it\") for a list, exactly like the button in " + "Mission Control: the connected ClaudeDo App creates the handler task and opens the triage " + "ConPTY tile for the given tasks. taskIds defaults to every Idle, non-manual task of the list. " + - "Requires the App to be running and connected to this worker -- the worker only broadcasts " + - "the request. taskIds accept task numbers (#123 or bare 123).")] + "Requires the App to be running and connected to this worker -- if it isn't, Requested comes " + + "back false with a reason and nothing is started. taskIds accept task numbers (#123 or bare 123).")] public async Task StartListHandler( [Description("The list to run the handler on.")] string listId, [Description("Tasks to hand to the run, in the order to triage them. Omit for all Idle, non-manual tasks of the list.")] IReadOnlyList? taskIds = null, @@ -65,8 +69,15 @@ public sealed class ListHandlerMcpTools if (chosen.Count == 0) throw new InvalidOperationException($"No tasks to hand to the list handler for {list.Name}."); + var numbers = chosen.Select(t => t.Number).ToList(); + if (!_clients.AnyConnected) + { + return new StartListHandlerResult(false, listId, list.Name, chosen.Count, numbers, + "No ClaudeDo App is connected to this worker, so the request could not be delivered. Open the app and retry."); + } + var ids = chosen.Select(t => t.Id).ToList(); await _broadcaster.ListHandlerStartRequested(listId, ids); - return new StartListHandlerResult(true, listId, list.Name, ids.Count, chosen.Select(t => t.Number).ToList()); + return new StartListHandlerResult(true, listId, list.Name, ids.Count, numbers); } } diff --git a/src/ClaudeDo.Worker/Hub/ConnectedClientTracker.cs b/src/ClaudeDo.Worker/Hub/ConnectedClientTracker.cs new file mode 100644 index 00000000..e3cc7a6f --- /dev/null +++ b/src/ClaudeDo.Worker/Hub/ConnectedClientTracker.cs @@ -0,0 +1,19 @@ +namespace ClaudeDo.Worker.Hub; + +/// +/// Counts SignalR clients (the ClaudeDo App) currently connected to . +/// A Hub instance is transient per connection, so this lives as a DI singleton and is +/// updated from / +/// — the only way an out-of-band caller (external MCP's start_list_handler, which only +/// broadcasts) can tell whether anything is listening before claiming it did something. +/// +public sealed class ConnectedClientTracker +{ + private int _count; + + public bool AnyConnected => Volatile.Read(ref _count) > 0; + + public void Increment() => Interlocked.Increment(ref _count); + + public void Decrement() => Interlocked.Decrement(ref _count); +} diff --git a/src/ClaudeDo.Worker/Hub/WorkerHub.cs b/src/ClaudeDo.Worker/Hub/WorkerHub.cs index 5e682e0a..5f8b24f0 100644 --- a/src/ClaudeDo.Worker/Hub/WorkerHub.cs +++ b/src/ClaudeDo.Worker/Hub/WorkerHub.cs @@ -74,6 +74,7 @@ public sealed class WorkerHub : Microsoft.AspNetCore.SignalR.Hub private readonly TicketImportService _ticketImport; private readonly HandlerRestartCoordinator? _handlerRestart; private readonly HandlerRoundState? _roundState; + private readonly ConnectedClientTracker? _clientTracker; public WorkerHub( QueueService queue, @@ -114,7 +115,8 @@ public sealed class WorkerHub : Microsoft.AspNetCore.SignalR.Hub TicketClientFactory ticketClients = null!, TicketImportService ticketImport = null!, HandlerRestartCoordinator? handlerRestart = null, - HandlerRoundState? roundState = null) + HandlerRoundState? roundState = null, + ConnectedClientTracker? clientTracker = null) { _queue = queue; _waker = waker; @@ -155,6 +157,21 @@ public sealed class WorkerHub : Microsoft.AspNetCore.SignalR.Hub _ticketImport = ticketImport; _handlerRestart = handlerRestart; _roundState = roundState; + _clientTracker = clientTracker; + } + + // Tracks connected UI clients so an out-of-band caller (external MCP's start_list_handler) + // can tell whether the broadcast it just sent had anyone to receive it. + public override Task OnConnectedAsync() + { + _clientTracker?.Increment(); + return base.OnConnectedAsync(); + } + + public override Task OnDisconnectedAsync(Exception? exception) + { + _clientTracker?.Decrement(); + return base.OnDisconnectedAsync(exception); } // Persistence boundary for the session_skills JSON-array columns (task/list/global). diff --git a/src/ClaudeDo.Worker/Program.cs b/src/ClaudeDo.Worker/Program.cs index 7724a690..70abb6bc 100644 --- a/src/ClaudeDo.Worker/Program.cs +++ b/src/ClaudeDo.Worker/Program.cs @@ -99,6 +99,7 @@ builder.Services.AddSignalR(options => options.MaximumParallelInvocationsPerClie // Runner stack. builder.Services.AddSingleton(); builder.Services.AddSingleton(); +builder.Services.AddSingleton(); builder.Services.AddSingleton(); builder.Services.AddSingleton(); builder.Services.AddSingleton(); diff --git a/tests/ClaudeDo.Worker.Tests/External/ListHandlerMcpToolsTests.cs b/tests/ClaudeDo.Worker.Tests/External/ListHandlerMcpToolsTests.cs new file mode 100644 index 00000000..de549054 --- /dev/null +++ b/tests/ClaudeDo.Worker.Tests/External/ListHandlerMcpToolsTests.cs @@ -0,0 +1,113 @@ +using ClaudeDo.Data; +using ClaudeDo.Data.Models; +using ClaudeDo.Data.Repositories; +using ClaudeDo.Worker.External; +using ClaudeDo.Worker.Hub; +using ClaudeDo.Worker.Tests.Infrastructure; +using TaskStatus = ClaudeDo.Data.Models.TaskStatus; + +namespace ClaudeDo.Worker.Tests.External; + +public sealed class ListHandlerMcpToolsTests : IDisposable +{ + private readonly DbFixture _db = new(); + private readonly ClaudeDoDbContext _ctx; + private readonly TaskRepository _tasks; + private readonly ListRepository _lists; + private readonly CapturingHubContext _hubContext = new(); + + public ListHandlerMcpToolsTests() + { + _ctx = _db.CreateContext(); + _tasks = new TaskRepository(_ctx); + _lists = new ListRepository(_ctx); + } + + public void Dispose() + { + _ctx.Dispose(); + _db.Dispose(); + } + + private ListHandlerMcpTools BuildSut(ConnectedClientTracker tracker) => + new(_lists, _tasks, new HubBroadcaster(_hubContext), tracker); + + private async Task SeedListAsync() + { + var listId = Guid.NewGuid().ToString(); + await _lists.AddAsync(new ListEntity { Id = listId, Name = "L", CreatedAt = DateTime.UtcNow }); + return listId; + } + + private async Task SeedTaskAsync(string listId, TaskStatus status = TaskStatus.Idle, string title = "t") + { + var task = new TaskEntity + { + Id = Guid.NewGuid().ToString(), ListId = listId, Title = title, + Status = status, CreatedAt = DateTime.UtcNow, CommitType = "chore", + }; + await _tasks.AddAsync(task); + return task; + } + + [Fact] + public async Task StartListHandler_NoConnectedClient_DoesNotBroadcastAndReportsNotRequested() + { + var listId = await SeedListAsync(); + await SeedTaskAsync(listId); + var tracker = new ConnectedClientTracker(); + + var result = await BuildSut(tracker).StartListHandler(listId, cancellationToken: CancellationToken.None); + + Assert.False(result.Requested); + Assert.False(string.IsNullOrWhiteSpace(result.Reason)); + Assert.Equal(1, result.TaskCount); + Assert.Empty(_hubContext.Proxy.Calls); + } + + [Fact] + public async Task StartListHandler_ConnectedClient_BroadcastsAndReportsRequested() + { + var listId = await SeedListAsync(); + var task = await SeedTaskAsync(listId); + var tracker = new ConnectedClientTracker(); + tracker.Increment(); + + var result = await BuildSut(tracker).StartListHandler(listId, cancellationToken: CancellationToken.None); + + Assert.True(result.Requested); + Assert.Null(result.Reason); + Assert.Equal(1, result.TaskCount); + Assert.Equal(new[] { task.Number }, result.TaskNumbers); + + var call = Assert.Single(_hubContext.Proxy.Calls); + Assert.Equal("ListHandlerStartRequested", call.Method); + Assert.Equal(listId, call.Args[0]); + } + + [Fact] + public async Task StartListHandler_LastClientDisconnects_StopsReportingRequested() + { + var listId = await SeedListAsync(); + await SeedTaskAsync(listId); + var tracker = new ConnectedClientTracker(); + tracker.Increment(); + tracker.Decrement(); + + var result = await BuildSut(tracker).StartListHandler(listId, cancellationToken: CancellationToken.None); + + Assert.False(result.Requested); + Assert.Empty(_hubContext.Proxy.Calls); + } + + [Fact] + public async Task StartListHandler_NoTasksToHandle_ThrowsRegardlessOfConnection() + { + var listId = await SeedListAsync(); + var tracker = new ConnectedClientTracker(); + tracker.Increment(); + + await Assert.ThrowsAsync(() => + BuildSut(tracker).StartListHandler(listId, cancellationToken: CancellationToken.None)); + } +}