Merge claudedo/21d704131b2d486cb1d103134e5ea5e8
This commit is contained in:
@@ -482,7 +482,7 @@ public static class PromptFiles
|
||||
- Running or WaitingForChildren → leave it; the wait below covers it.
|
||||
- WaitingForReview → leave it; it goes straight to Phase 4.
|
||||
|
||||
Then wait with wait_for_task_change instead of sleeping and polling get_task yourself. Pass the ids of every task not yet in WaitingForReview or a terminal status — Queued, Running and WaitingForChildren alike — and set treatWaitingForChildrenAsBusy=true. Without that flag a task with children returns the moment it goes Running → WaitingForChildren, while its children are still working, and you would walk into Phase 4 with unfinished work. Use timeoutSeconds 900: the server clamps there anyway, and ClaudeDo's launchers already raise MCP_TOOL_TIMEOUT above it, so one long wait costs one turn where six short ones cost six.
|
||||
Then wait with wait_for_task_change instead of sleeping and polling get_task yourself. Pass the ids of every task not yet in WaitingForReview or a terminal status — Queued, Running and WaitingForChildren alike — and set treatWaitingForChildrenAsBusy=true. Without that flag a task with children returns the moment it goes Running → WaitingForChildren, while its children are still working, and you would walk into Phase 4 with unfinished work. Use timeoutSeconds 900: the server clamps there anyway, and it pings MCP progress every 30s while waiting so one long wait survives your client's own idle-silence abort and costs one turn where six short ones cost six.
|
||||
|
||||
It returns as soon as a task reaches WaitingForReview or fails, or reports timedOut if none did. Report progress as tasks land, then call it again with whatever ids are still outstanding until none remain.
|
||||
|
||||
|
||||
+27
-6
@@ -1,6 +1,7 @@
|
||||
using System.ComponentModel;
|
||||
using ClaudeDo.Data;
|
||||
using Microsoft.EntityFrameworkCore;
|
||||
using ModelContextProtocol;
|
||||
using ModelContextProtocol.Server;
|
||||
using TaskStatus = ClaudeDo.Data.Models.TaskStatus;
|
||||
|
||||
@@ -15,12 +16,22 @@ public sealed class TaskWaitMcpTools
|
||||
// Every ClaudeDo-owned launcher (ClaudeProcess for headless runs, InteractiveLaunchSpecService
|
||||
// for ConPTY sessions) sets MCP_TOOL_TIMEOUT=930000ms on the claude CLI process; this cap
|
||||
// leaves a ~30s margin under that so the tool itself reports TimedOut instead of racing the
|
||||
// client's own abort. A caller running claude with a different MCP_TOOL_TIMEOUT (or none --
|
||||
// the CLI default is 60s) will see its own client-side timeout fire first; this tool has no
|
||||
// way to detect or compensate for that from the server side.
|
||||
// client's own wall-clock abort. A caller running claude with a different MCP_TOOL_TIMEOUT (or
|
||||
// none -- the CLI default is 60s) will see its own client-side timeout fire first; this tool
|
||||
// has no way to detect or compensate for that from the server side.
|
||||
//
|
||||
// MCP_TOOL_TIMEOUT is a separate mechanism from Claude Code's idle-silence abort
|
||||
// (CLAUDE_CODE_MCP_TOOL_IDLE_TIMEOUT, default 300s for HTTP-transport MCP servers like this
|
||||
// one) -- no ClaudeDo launcher raises that one. Without periodic activity, a wait anywhere
|
||||
// near this cap gets killed at ~300s regardless of launcher. WaitForTaskChange survives that
|
||||
// by reporting MCP progress well under 300s apart (see ProgressReportInterval below), which
|
||||
// resets Claude Code's idle timer.
|
||||
internal const int MaxTimeoutSeconds = 900;
|
||||
private static readonly TimeSpan PollInterval = TimeSpan.FromMilliseconds(500);
|
||||
|
||||
// Not readonly: tests shrink this to observe a progress report without waiting 30s.
|
||||
internal static TimeSpan ProgressReportInterval = TimeSpan.FromSeconds(30);
|
||||
|
||||
private readonly IDbContextFactory<ClaudeDoDbContext> _dbFactory;
|
||||
|
||||
public TaskWaitMcpTools(IDbContextFactory<ClaudeDoDbContext> dbFactory)
|
||||
@@ -33,9 +44,10 @@ public sealed class TaskWaitMcpTools
|
||||
"polling get_task in a loop. Returns immediately if a task is already outside Queued/Running " +
|
||||
"(an unknown id reports status \"NotFound\" and counts as changed). Pitfall: a planning parent " +
|
||||
"goes Running -> WaitingForChildren while its children are still working, so by default " +
|
||||
"waiting on a parent returns early; see treatWaitingForChildrenAsBusy. Requires the calling " +
|
||||
"claude process to run with MCP_TOOL_TIMEOUT >= 930000 (ms) for a long wait to actually be " +
|
||||
"held open -- ClaudeDo's own launchers already set this.")]
|
||||
"waiting on a parent returns early; see treatWaitingForChildrenAsBusy. Sends MCP progress " +
|
||||
"pings every 30s while waiting so a long wait survives the calling client's own idle-silence " +
|
||||
"abort (Claude Code defaults to killing an MCP call after ~300s of silence) -- this is not " +
|
||||
"guaranteed by every possible MCP client.")]
|
||||
public async Task<WaitForTaskChangeResult> WaitForTaskChange(
|
||||
string[] taskIds,
|
||||
[Description(
|
||||
@@ -47,6 +59,7 @@ public sealed class TaskWaitMcpTools
|
||||
"continues until it reaches WaitingForReview or a terminal status instead of returning " +
|
||||
"as soon as it leaves Running.")]
|
||||
bool treatWaitingForChildrenAsBusy = false,
|
||||
IProgress<ProgressNotificationValue>? progress = null,
|
||||
CancellationToken cancellationToken = default)
|
||||
{
|
||||
if (taskIds.Length == 0)
|
||||
@@ -58,12 +71,20 @@ public sealed class TaskWaitMcpTools
|
||||
|
||||
try
|
||||
{
|
||||
var lastProgressAt = DateTime.UtcNow;
|
||||
while (true)
|
||||
{
|
||||
var changed = await CheckOnceAsync(taskIds, treatWaitingForChildrenAsBusy, linked.Token);
|
||||
if (changed.Count > 0)
|
||||
return new WaitForTaskChangeResult(changed, TimedOut: false);
|
||||
|
||||
var now = DateTime.UtcNow;
|
||||
if (now - lastProgressAt >= ProgressReportInterval)
|
||||
{
|
||||
lastProgressAt = now;
|
||||
progress?.Report(new ProgressNotificationValue { Progress = 0, Message = "Still waiting for a task status change." });
|
||||
}
|
||||
|
||||
await Task.Delay(PollInterval, linked.Token);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -4,6 +4,7 @@ using ClaudeDo.Data.Models;
|
||||
using ClaudeDo.Data.Repositories;
|
||||
using ClaudeDo.Worker.External;
|
||||
using ClaudeDo.Worker.Tests.Infrastructure;
|
||||
using ModelContextProtocol;
|
||||
using TaskStatus = ClaudeDo.Data.Models.TaskStatus;
|
||||
|
||||
namespace ClaudeDo.Worker.Tests.External;
|
||||
@@ -202,4 +203,43 @@ public sealed class TaskWaitMcpToolsTests : IDisposable
|
||||
// ClaudeProcess / InteractiveLaunchSpecService set MCP_TOOL_TIMEOUT=930000ms.
|
||||
Assert.True(TaskWaitMcpTools.MaxTimeoutSeconds < 930);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task WaitForTaskChange_LongWait_ReportsProgressWellUnderClaudeCodeIdleTimeout()
|
||||
{
|
||||
var original = TaskWaitMcpTools.ProgressReportInterval;
|
||||
TaskWaitMcpTools.ProgressReportInterval = TimeSpan.FromMilliseconds(200);
|
||||
try
|
||||
{
|
||||
var task = await SeedTaskAsync(TaskStatus.Running);
|
||||
var sut = BuildSut();
|
||||
var reports = new List<ProgressNotificationValue>();
|
||||
var progress = new Progress<ProgressNotificationValue>(reports.Add);
|
||||
|
||||
var result = await sut.WaitForTaskChange(
|
||||
[task.Id], timeoutSeconds: 1, progress: progress, cancellationToken: CancellationToken.None);
|
||||
|
||||
Assert.True(result.TimedOut);
|
||||
// Progress<T> marshals via the SynchronizationContext captured at construction; give
|
||||
// any queued callbacks a beat to run before asserting on `reports`.
|
||||
await Task.Delay(200);
|
||||
Assert.NotEmpty(reports);
|
||||
}
|
||||
finally
|
||||
{
|
||||
TaskWaitMcpTools.ProgressReportInterval = original;
|
||||
}
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task WaitForTaskChange_NoProgressHandlerPassed_DoesNotThrow()
|
||||
{
|
||||
var task = await SeedTaskAsync(TaskStatus.Running);
|
||||
var sut = BuildSut();
|
||||
|
||||
var result = await sut.WaitForTaskChange(
|
||||
[task.Id], timeoutSeconds: 1, progress: null, cancellationToken: CancellationToken.None);
|
||||
|
||||
Assert.True(result.TimedOut);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user