releases/ClaudeDo
2
0
Fork 0
Code Issues 1 Pull Requests 2 Actions Packages Projects Releases 24 Wiki Activity

fix(worker): cap the list-handler wait/merge handoff chain server-side

Browse Source 
HandoffMcpTools.HandoffListHandler only validated the phase name and
broadcast it -- nothing bounded how many times a ConPTY session could
hand off wait<->merge, so a model that skimmed past the prompt's
"final round" line could loop indefinitely. Add HandoffRoundTracker,
an in-memory per-handler-task counter (a list-handler run always
creates a fresh handler task, so no reset logic is needed): past 4
handoffs (two full wait/merge cycles) a non-final nextPhase is
coerced to its "_final" counterpart, and any handoff after a
merge_final round for that task is rejected outright.
This commit is contained in:
mika kuns
2026-08-11 16:52:49 +02:00
parent 79b35801ae
commit 39b24c2f74
4 changed files with 156 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/ClaudeDo.Worker/External/HandoffMcpTools.cs
Vendored
+18 -5
View File
@@ -6,18 +6,23 @@ using ModelContextProtocol.Server;
namespace ClaudeDo.Worker.External;
public sealed record HandoffListHandlerResult(bool Requested, string TaskId, int Number, int SurvivingCount, string NextPhase);
// Coerced=true means nextPhase asked for a non-final phase but the server-side round cap
// (HandoffRoundTracker) forced it to its "_final" counterpart instead -- NextPhase already
// reflects the effective (post-coercion) value in that case.
public sealed record HandoffListHandlerResult(bool Requested, string TaskId, int Number, int SurvivingCount, string NextPhase, bool Coerced = false);
[McpServerToolType]
public sealed class HandoffMcpTools
{
private readonly TaskRepository _tasks;
private readonly HubBroadcaster _broadcaster;
private readonly HandoffRoundTracker _rounds;
public HandoffMcpTools(TaskRepository tasks, HubBroadcaster broadcaster)
public HandoffMcpTools(TaskRepository tasks, HubBroadcaster broadcaster, HandoffRoundTracker rounds)
{
_tasks = tasks;
_broadcaster = broadcaster;
_rounds = rounds;
}
[McpServerTool, Description(
@@ -27,7 +32,11 @@ public sealed class HandoffMcpTools
"untouched. nextPhase picks the next session's role: \"wait\" (queue + wait; the normal handoff " +
"after triage) or \"merge\" (review + merge; the normal handoff after wait) -- \"wait_final\"/" +
"\"merge_final\" are only for a merge session that started reruns and must eventually stop the " +
"chain instead of restarting again. An unrecognized value is rejected. The current tile stays " +
"chain instead of restarting again. An unrecognized value is rejected. This handoff chain is " +
"capped server-side at " + nameof(HandoffRoundTracker.MaxHandoffs) + " (four handoffs, i.e. two " +
"full wait/merge cycles) for a given handler task: once reached, a \"wait\"/\"merge\" request is " +
"silently coerced to its \"_final\" counterpart instead (check the result's coerced field), and " +
"any handoff at all after a merge_final round for this task throws. The current tile stays " +
"open; you must end your own turn immediately after calling this.")]
public async Task<HandoffListHandlerResult> HandoffListHandler(
[Description("This session's own handler task id.")] string taskId,
@@ -50,7 +59,11 @@ public sealed class HandoffMcpTools
_ = await _tasks.GetByIdAsync(id, cancellationToken)
?? throw new InvalidOperationException($"Task {id} not found.");
await _broadcaster.HandoffRequested(taskId, survivingTaskIds, nextPhase);
return new HandoffListHandlerResult(true, taskId, task.Number, survivingTaskIds.Count, nextPhase);
var round = _rounds.Register(taskId, nextPhase);
if (round.Rejected)
throw new InvalidOperationException(round.Error);
await _broadcaster.HandoffRequested(taskId, survivingTaskIds, round.EffectivePhase);
return new HandoffListHandlerResult(true, taskId, task.Number, survivingTaskIds.Count, round.EffectivePhase, round.Coerced);
}
}
src/ClaudeDo.Worker/External/HandoffRoundTracker.cs
Vendored
+54
View File
@@ -0,0 +1,54 @@
using System.Collections.Concurrent;
using ClaudeDo.Worker.Runner;
namespace ClaudeDo.Worker.External;
// Bounds the list-handler wait<->merge handoff chain server-side, so a model that ignores the
// cooperative "this is the FINAL round" line in handoff.md (InteractiveLaunchSpecService's
// finalNote) cannot loop wait->merge->wait->merge forever, each round spawning a fresh ConPTY
// session. Keyed by handler task id -- a "Let Claude handle it" run always creates a fresh
// handler task (InteractiveLaunchSpecService.CreateMergeHelperTaskAsync), so the counter is
// per-run by construction: no reset logic needed, and a later run on the same list starts at 0
// on its own. In-memory only, same tradeoff as TaskRunTokenRegistry -- a worker restart mid-run
// resets the count, which is acceptable since the chain restarts fresh anyway.
public sealed class HandoffRoundTracker
{
// Two full wait->merge cycles: wait, merge, wait, merge.
public const int MaxHandoffs = 4;
private sealed class TaskState
{
public int Count;
public bool Sealed;
}
private readonly ConcurrentDictionary<string, TaskState> _states = new();
public sealed record Result(bool Rejected, bool Coerced, string EffectivePhase, string? Error);
public Result Register(string taskId, string nextPhase)
{
var state = _states.GetOrAdd(taskId, _ => new TaskState());
lock (state)
{
if (state.Sealed)
return new Result(true, false, nextPhase,
$"Task {taskId} already completed its merge_final round -- no further handoff is allowed.");
state.Count++;
var isFinal = nextPhase is MergeHelperPhase.WaitFinal or MergeHelperPhase.MergeFinal;
var effective = nextPhase;
if (state.Count > MaxHandoffs && !isFinal)
{
effective = nextPhase == MergeHelperPhase.Wait ? MergeHelperPhase.WaitFinal : MergeHelperPhase.MergeFinal;
isFinal = true;
}
if (isFinal && effective == MergeHelperPhase.MergeFinal)
state.Sealed = true;
return new Result(false, !string.Equals(effective, nextPhase, StringComparison.Ordinal), effective, null);
}
}
}
src/ClaudeDo.Worker/Program.cs
+1
View File
@@ -317,6 +317,7 @@ if (cfg.ExternalMcpPort > 0)
externalBuilder.Services.AddScoped<RunHistoryMcpTools>();
externalBuilder.Services.AddScoped<AgentMcpTools>();
externalBuilder.Services.AddScoped<LifecycleMcpTools>();
externalBuilder.Services.AddSingleton<HandoffRoundTracker>();
externalBuilder.Services.AddScoped<HandoffMcpTools>();
externalBuilder.Services.AddScoped<AppSettingsMcpTools>();
externalBuilder.Services.AddScoped<TaskWaitMcpTools>();
tests/ClaudeDo.Worker.Tests/External/HandoffMcpToolsTests.cs
Vendored
+83 -1
View File
@@ -25,7 +25,8 @@ public sealed class HandoffMcpToolsTests : IDisposable
public void Dispose() { _ctx.Dispose(); _db.Dispose(); }
private HandoffMcpTools BuildSut() => new(_tasks, new HubBroadcaster(_hubContext));
private HandoffMcpTools BuildSut(HandoffRoundTracker? tracker = null) =>
new(_tasks, new HubBroadcaster(_hubContext), tracker ?? new HandoffRoundTracker());
private async Task<TaskEntity> SeedTaskAsync(string listId, TaskStatus status = TaskStatus.Idle, string title = "t")
{
@@ -138,4 +139,85 @@ public sealed class HandoffMcpToolsTests : IDisposable
await Assert.ThrowsAsync<InvalidOperationException>(() =>
sut.HandoffListHandler(handlerTask.Id, new[] { "missing" }, cancellationToken: CancellationToken.None));
}
[Fact]
public async Task HandoffListHandler_NormalWaitMergeWaitFinalMergeFinalSequence_PassesUntouched()
{
var listId = await SeedListAsync();
var handlerTask = await SeedTaskAsync(listId);
var survivor = await SeedTaskAsync(listId, TaskStatus.WaitingForReview, title: "Survivor");
var tracker = new HandoffRoundTracker();
var survivors = new[] { survivor.Id };
foreach (var phase in new[] { "wait", "merge", "wait_final", "merge_final" })
{
var sut = BuildSut(tracker);
var result = await sut.HandoffListHandler(handlerTask.Id, survivors, phase, CancellationToken.None);
Assert.False(result.Coerced);
Assert.Equal(phase, result.NextPhase);
}
}
[Fact]
public async Task HandoffListHandler_PastCap_CoercesNonFinalPhaseToItsFinalCounterpart()
{
var listId = await SeedListAsync();
var handlerTask = await SeedTaskAsync(listId);
var survivor = await SeedTaskAsync(listId, TaskStatus.WaitingForReview, title: "Survivor");
var tracker = new HandoffRoundTracker();
var survivors = new[] { survivor.Id };
foreach (var phase in new[] { "wait", "merge", "wait", "merge" })
{
var sut = BuildSut(tracker);
var result = await sut.HandoffListHandler(handlerTask.Id, survivors, phase, CancellationToken.None);
Assert.False(result.Coerced);
}
var fifthSut = BuildSut(tracker);
var fifth = await fifthSut.HandoffListHandler(handlerTask.Id, survivors, "wait", CancellationToken.None);
Assert.True(fifth.Coerced);
Assert.Equal("wait_final", fifth.NextPhase);
}
[Fact]
public async Task HandoffListHandler_AfterMergeFinal_AnyFurtherHandoffThrows()
{
var listId = await SeedListAsync();
var handlerTask = await SeedTaskAsync(listId);
var survivor = await SeedTaskAsync(listId, TaskStatus.WaitingForReview, title: "Survivor");
var tracker = new HandoffRoundTracker();
var survivors = new[] { survivor.Id };
var firstSut = BuildSut(tracker);
await firstSut.HandoffListHandler(handlerTask.Id, survivors, "merge_final", CancellationToken.None);
var secondSut = BuildSut(tracker);
await Assert.ThrowsAsync<InvalidOperationException>(() =>
secondSut.HandoffListHandler(handlerTask.Id, survivors, "wait", CancellationToken.None));
}
[Fact]
public async Task HandoffListHandler_TwoHandlerTasks_DoNotShareACounter()
{
var listId = await SeedListAsync();
var handlerA = await SeedTaskAsync(listId, title: "Handler A");
var handlerB = await SeedTaskAsync(listId, title: "Handler B");
var survivorA = await SeedTaskAsync(listId, TaskStatus.WaitingForReview, title: "Survivor A");
var survivorB = await SeedTaskAsync(listId, TaskStatus.WaitingForReview, title: "Survivor B");
var tracker = new HandoffRoundTracker();
foreach (var phase in new[] { "wait", "merge", "wait", "merge" })
{
var sut = BuildSut(tracker);
await sut.HandoffListHandler(handlerA.Id, new[] { survivorA.Id }, phase, CancellationToken.None);
}
var sutB = BuildSut(tracker);
var resultB = await sutB.HandoffListHandler(handlerB.Id, new[] { survivorB.Id }, "wait", CancellationToken.None);
Assert.False(resultB.Coerced);
Assert.Equal("wait", resultB.NextPhase);
}
}