feat(claude-do): merge Handoff-Budget im Brief sichtbar machen und pro Task mitzähl
ClaudeDo-Task: bdcd601a-2fed-44d8-9164-6d2d008e7f31
This commit is contained in:
@@ -617,7 +617,7 @@ public static class PromptFiles
|
||||
|
||||
Start with phase 3 (run), continuing through review/merge and the summary as your
|
||||
instructions describe.
|
||||
{finalNote}
|
||||
{roundInfo}
|
||||
""";
|
||||
|
||||
private const string WeeklyReportDefault = """
|
||||
|
||||
+15
-3
@@ -5,8 +5,8 @@ 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
|
||||
// roundInfo, read back via GetState) 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
|
||||
@@ -26,6 +26,17 @@ public sealed class HandoffRoundTracker
|
||||
|
||||
public sealed record Result(bool Rejected, bool Coerced, string EffectivePhase, string? Error);
|
||||
|
||||
/// <summary>Read-only snapshot for a handler task -- does NOT register a round. Used to report
|
||||
/// the current round/remaining budget (e.g. in the handoff brief) without mutating state that
|
||||
/// only Register should advance. Count is 0 for a task that hasn't handed off yet.</summary>
|
||||
public (int Count, bool Sealed) GetState(string taskId)
|
||||
{
|
||||
if (!_states.TryGetValue(taskId, out var state))
|
||||
return (0, false);
|
||||
lock (state)
|
||||
return (state.Count, state.Sealed);
|
||||
}
|
||||
|
||||
public Result Register(string taskId, string nextPhase)
|
||||
{
|
||||
var state = _states.GetOrAdd(taskId, _ => new TaskState());
|
||||
@@ -33,7 +44,8 @@ public sealed class HandoffRoundTracker
|
||||
{
|
||||
if (state.Sealed)
|
||||
return new Result(true, false, nextPhase,
|
||||
$"Task {taskId} already completed its merge_final round -- no further handoff is allowed.");
|
||||
$"Task {taskId} already completed its merge_final round -- no further handoff is allowed. " +
|
||||
"Start a new handler run (\"Let Claude handle it\") on this list instead.");
|
||||
|
||||
state.Count++;
|
||||
var isFinal = nextPhase is MergeHelperPhase.WaitFinal or MergeHelperPhase.MergeFinal;
|
||||
|
||||
@@ -94,6 +94,7 @@ builder.Services.AddSingleton<PendingQuestionRegistry>();
|
||||
builder.Services.AddSingleton<IRepoCloner, GitRepoCloner>();
|
||||
builder.Services.AddSingleton<ISessionSkillRegistry, SessionSkillRegistry>();
|
||||
builder.Services.AddSingleton<ISessionSkillSeeder, SessionSkillSeeder>();
|
||||
builder.Services.AddSingleton<HandoffRoundTracker>();
|
||||
builder.Services.AddSingleton<InteractiveLaunchSpecService>();
|
||||
builder.Services.AddSingleton<TaskRunner>();
|
||||
builder.Services.AddSingleton<WorktreeMaintenanceService>();
|
||||
@@ -335,6 +336,7 @@ if (cfg.ExternalMcpPort > 0)
|
||||
Share<InteractiveReviewSubmissionService>();
|
||||
Share<AttachmentStore>();
|
||||
Share<FindingsStore>();
|
||||
Share<HandoffRoundTracker>();
|
||||
|
||||
externalBuilder.Services.AddScoped<ClaudeDoDbContext>(sp =>
|
||||
sp.GetRequiredService<IDbContextFactory<ClaudeDoDbContext>>().CreateDbContext());
|
||||
@@ -342,7 +344,6 @@ if (cfg.ExternalMcpPort > 0)
|
||||
externalBuilder.Services.AddScoped<ListRepository>();
|
||||
externalBuilder.Services.AddScoped<TaskRunRepository>();
|
||||
externalBuilder.Services.AddScoped<TaskAttachmentRepository>();
|
||||
externalBuilder.Services.AddSingleton<HandoffRoundTracker>();
|
||||
externalBuilder.Services.AddScoped<FindingsStoreLocator>();
|
||||
externalBuilder.Services.AddScoped<ExternalMcpService>();
|
||||
externalBuilder.Services.AddScoped<MergeMcpTools>();
|
||||
|
||||
@@ -4,6 +4,7 @@ using ClaudeDo.Data.Git;
|
||||
using ClaudeDo.Data.Models;
|
||||
using ClaudeDo.Data.Repositories;
|
||||
using ClaudeDo.Worker.Config;
|
||||
using ClaudeDo.Worker.External;
|
||||
using ClaudeDo.Worker.Planning;
|
||||
using ClaudeDo.Worker.Skills;
|
||||
using Microsoft.EntityFrameworkCore;
|
||||
@@ -42,6 +43,7 @@ public sealed class InteractiveLaunchSpecService
|
||||
private readonly WorktreeManager _wtManager;
|
||||
private readonly GitService _git;
|
||||
private readonly string _claudePath;
|
||||
private readonly HandoffRoundTracker _handoffRounds;
|
||||
|
||||
public InteractiveLaunchSpecService(
|
||||
IDbContextFactory<ClaudeDoDbContext> dbFactory,
|
||||
@@ -49,7 +51,8 @@ public sealed class InteractiveLaunchSpecService
|
||||
ISessionSkillRegistry skillRegistry,
|
||||
WorktreeManager wtManager,
|
||||
GitService git,
|
||||
WorkerConfig cfg)
|
||||
WorkerConfig cfg,
|
||||
HandoffRoundTracker handoffRounds)
|
||||
{
|
||||
_dbFactory = dbFactory;
|
||||
_skillSeeder = skillSeeder;
|
||||
@@ -57,6 +60,7 @@ public sealed class InteractiveLaunchSpecService
|
||||
_wtManager = wtManager;
|
||||
_git = git;
|
||||
_claudePath = cfg.ClaudeBin;
|
||||
_handoffRounds = handoffRounds;
|
||||
}
|
||||
|
||||
/// <summary>Builds a LaunchSpec for opening an interactive session in a task's worktree.
|
||||
@@ -393,13 +397,19 @@ public sealed class InteractiveLaunchSpecService
|
||||
var systemPromptPath = Path.Combine(sessionDir, "system-prompt.md");
|
||||
await File.WriteAllTextAsync(systemPromptPath, PromptFiles.ReadOrDefault(phase.PromptKind), ct);
|
||||
|
||||
// A "_final" round differs from its non-final counterpart in exactly one rendered line --
|
||||
// the same system prompt and model drive both -- marking the final round and forbidding a
|
||||
// further rerun, so the chain is bounded structurally rather than by prompt appeal alone.
|
||||
var finalNote = phase.IsFinal
|
||||
? "\nThis is the FINAL round of this list-handler run -- do not start any further reruns; " +
|
||||
"merge what you can, report the rest, and print the summary."
|
||||
: "";
|
||||
// The round count/cap come from HandoffRoundTracker -- the same instance HandoffMcpTools
|
||||
// registered this handoff against -- never recomputed here, so the brief can't drift from
|
||||
// the server-side cap that actually enforces the chain. A "_final" round adds one more line
|
||||
// forbidding a further rerun, so the chain is bounded structurally rather than by prompt
|
||||
// appeal alone.
|
||||
var round = _handoffRounds.GetState(taskId);
|
||||
var remaining = Math.Max(HandoffRoundTracker.MaxHandoffs - round.Count, 0);
|
||||
var roundInfo = $"Handoff budget: this is round {round.Count} of " +
|
||||
$"{HandoffRoundTracker.MaxHandoffs} possible handoffs for this list-handler run " +
|
||||
$"({remaining} more possible after this one).";
|
||||
if (phase.IsFinal)
|
||||
roundInfo += "\nThis is the FINAL round of this list-handler run -- do not start any further reruns; " +
|
||||
"merge what you can, report the rest, and print the summary.";
|
||||
|
||||
var briefPath = Path.Combine(sessionDir, "handoff.md");
|
||||
await File.WriteAllTextAsync(briefPath, PromptFiles.Render(PromptKind.MergeHelperHandoff,
|
||||
@@ -409,7 +419,7 @@ public sealed class InteractiveLaunchSpecService
|
||||
["repo"] = repoDir,
|
||||
["handlerTaskId"] = taskId,
|
||||
["tasks"] = RenderGroupedBrief(briefEntries),
|
||||
["finalNote"] = finalNote,
|
||||
["roundInfo"] = roundInfo,
|
||||
}), ct);
|
||||
|
||||
var resolvedClaude = WindowsTerminalLauncher.Resolve(_claudePath)
|
||||
|
||||
@@ -198,6 +198,63 @@ public sealed class HandoffMcpToolsTests : IDisposable
|
||||
secondSut.HandoffListHandler(handlerTask.Id, survivors, "wait", CancellationToken.None));
|
||||
}
|
||||
|
||||
[Theory]
|
||||
[InlineData(1, 3)]
|
||||
[InlineData(2, 2)]
|
||||
[InlineData(4, 0)]
|
||||
public void HandoffRoundTracker_GetState_ReportsCountAfterNRegistrations(int registrations, int expectedRemaining)
|
||||
{
|
||||
var tracker = new HandoffRoundTracker();
|
||||
var taskId = Guid.NewGuid().ToString();
|
||||
|
||||
for (var i = 0; i < registrations; i++)
|
||||
tracker.Register(taskId, "wait");
|
||||
|
||||
var (count, sealedState) = tracker.GetState(taskId);
|
||||
Assert.Equal(registrations, count);
|
||||
Assert.Equal(expectedRemaining, HandoffRoundTracker.MaxHandoffs - count);
|
||||
Assert.False(sealedState);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void HandoffRoundTracker_GetState_UnregisteredTask_ReturnsZero()
|
||||
{
|
||||
var tracker = new HandoffRoundTracker();
|
||||
var (count, sealedState) = tracker.GetState(Guid.NewGuid().ToString());
|
||||
Assert.Equal(0, count);
|
||||
Assert.False(sealedState);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void HandoffRoundTracker_Register_DoesNotMutateStateVisibleFromGetState_BeyondItsOwnCall()
|
||||
{
|
||||
var tracker = new HandoffRoundTracker();
|
||||
var taskId = Guid.NewGuid().ToString();
|
||||
|
||||
tracker.Register(taskId, "wait");
|
||||
var (beforeCount, _) = tracker.GetState(taskId);
|
||||
Assert.Equal(1, beforeCount);
|
||||
|
||||
// GetState itself must be read-only -- calling it repeatedly must not advance the round.
|
||||
tracker.GetState(taskId);
|
||||
tracker.GetState(taskId);
|
||||
var (afterCount, _) = tracker.GetState(taskId);
|
||||
Assert.Equal(beforeCount, afterCount);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void HandoffRoundTracker_RejectedHandoff_ErrorNamesTheNextAction()
|
||||
{
|
||||
var tracker = new HandoffRoundTracker();
|
||||
var taskId = Guid.NewGuid().ToString();
|
||||
tracker.Register(taskId, "merge_final");
|
||||
|
||||
var result = tracker.Register(taskId, "wait");
|
||||
|
||||
Assert.True(result.Rejected);
|
||||
Assert.Contains("new handler run", result.Error, StringComparison.OrdinalIgnoreCase);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task HandoffListHandler_TwoHandlerTasks_DoNotShareACounter()
|
||||
{
|
||||
|
||||
@@ -64,7 +64,7 @@ public sealed class MergeHelperTaskHubTests : IDisposable
|
||||
var wtManager = new WorktreeManager(git, factory, new WorkerConfig(), NullLogger<WorktreeManager>.Instance);
|
||||
var interactiveLaunchSpec = new InteractiveLaunchSpecService(
|
||||
factory, new FakeSessionSkillSeeder(), new FakeSessionSkillRegistry(), wtManager, git,
|
||||
new WorkerConfig { ClaudeBin = "claude" });
|
||||
new WorkerConfig { ClaudeBin = "claude" }, new ClaudeDo.Worker.External.HandoffRoundTracker());
|
||||
var built = TaskStateServiceBuilder.Build(factory);
|
||||
var reviewSubmission = new InteractiveReviewSubmissionService(factory, git, built.State, wtManager);
|
||||
|
||||
|
||||
@@ -3,6 +3,7 @@ using ClaudeDo.Data.Git;
|
||||
using ClaudeDo.Data.Models;
|
||||
using ClaudeDo.Data.Repositories;
|
||||
using ClaudeDo.Worker.Config;
|
||||
using ClaudeDo.Worker.External;
|
||||
using ClaudeDo.Worker.Planning;
|
||||
using ClaudeDo.Worker.Runner;
|
||||
using ClaudeDo.Worker.Skills;
|
||||
@@ -72,11 +73,12 @@ public sealed class InteractiveLaunchSpecServiceTests : IDisposable
|
||||
=> Task.FromResult<IReadOnlyList<SessionSkillEntity>>(Installed);
|
||||
}
|
||||
|
||||
private InteractiveLaunchSpecService BuildService() =>
|
||||
private InteractiveLaunchSpecService BuildService(HandoffRoundTracker? handoffRounds = null) =>
|
||||
new(_db.CreateFactory(), _seeder, _registry,
|
||||
new WorktreeManager(new GitService(), _db.CreateFactory(), new WorkerConfig(), NullLogger<WorktreeManager>.Instance),
|
||||
new GitService(),
|
||||
new WorkerConfig { ClaudeBin = _claudeStubPath });
|
||||
new WorkerConfig { ClaudeBin = _claudeStubPath },
|
||||
handoffRounds ?? new HandoffRoundTracker());
|
||||
|
||||
private async Task<string> SeedListAsync(string? workingDir = null, string name = "L")
|
||||
{
|
||||
@@ -1043,7 +1045,13 @@ public sealed class InteractiveLaunchSpecServiceTests : IDisposable
|
||||
|
||||
var beforeCount = await CountTasksAsync();
|
||||
|
||||
var svc = BuildService();
|
||||
// In production, HandoffMcpTools.HandoffListHandler calls Register BEFORE the hub asks
|
||||
// this service to build the spec -- simulate that here so the brief's round info is
|
||||
// demonstrably read from HandoffRoundTracker, not recomputed.
|
||||
var tracker = new HandoffRoundTracker();
|
||||
tracker.Register(handlerTaskId, nextPhase);
|
||||
|
||||
var svc = BuildService(tracker);
|
||||
var spec = await svc.BuildForMergeHelperHandoffAsync(handlerTaskId, new[] { survivor }, nextPhase, CancellationToken.None);
|
||||
var sessionDir = TrackSessionDir(spec);
|
||||
|
||||
@@ -1090,6 +1098,10 @@ public sealed class InteractiveLaunchSpecServiceTests : IDisposable
|
||||
Assert.Contains(survivor, handoffBrief);
|
||||
Assert.Contains("phase 3", handoffBrief, StringComparison.OrdinalIgnoreCase);
|
||||
|
||||
// Round info always names the current round and the total cap -- this is the ONE
|
||||
// Register call above, so round 1 of MaxHandoffs.
|
||||
Assert.Contains($"round 1 of {HandoffRoundTracker.MaxHandoffs}", handoffBrief);
|
||||
|
||||
if (expectFinalNote)
|
||||
{
|
||||
Assert.Contains("FINAL round", handoffBrief);
|
||||
|
||||
Reference in New Issue
Block a user