diff --git a/src/ClaudeDo.Data/PromptFiles.cs b/src/ClaudeDo.Data/PromptFiles.cs index 0c67850a..fed4518c 100644 --- a/src/ClaudeDo.Data/PromptFiles.cs +++ b/src/ClaudeDo.Data/PromptFiles.cs @@ -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 = """ diff --git a/src/ClaudeDo.Worker/External/HandoffRoundTracker.cs b/src/ClaudeDo.Worker/External/HandoffRoundTracker.cs index 56516242..92f6f63e 100644 --- a/src/ClaudeDo.Worker/External/HandoffRoundTracker.cs +++ b/src/ClaudeDo.Worker/External/HandoffRoundTracker.cs @@ -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); + /// 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. + 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; diff --git a/src/ClaudeDo.Worker/Program.cs b/src/ClaudeDo.Worker/Program.cs index 942e2d83..36d68698 100644 --- a/src/ClaudeDo.Worker/Program.cs +++ b/src/ClaudeDo.Worker/Program.cs @@ -94,6 +94,7 @@ builder.Services.AddSingleton(); builder.Services.AddSingleton(); builder.Services.AddSingleton(); builder.Services.AddSingleton(); +builder.Services.AddSingleton(); builder.Services.AddSingleton(); builder.Services.AddSingleton(); builder.Services.AddSingleton(); @@ -335,6 +336,7 @@ if (cfg.ExternalMcpPort > 0) Share(); Share(); Share(); + Share(); externalBuilder.Services.AddScoped(sp => sp.GetRequiredService>().CreateDbContext()); @@ -342,7 +344,6 @@ if (cfg.ExternalMcpPort > 0) externalBuilder.Services.AddScoped(); externalBuilder.Services.AddScoped(); externalBuilder.Services.AddScoped(); - externalBuilder.Services.AddSingleton(); externalBuilder.Services.AddScoped(); externalBuilder.Services.AddScoped(); externalBuilder.Services.AddScoped(); diff --git a/src/ClaudeDo.Worker/Runner/InteractiveLaunchSpecService.cs b/src/ClaudeDo.Worker/Runner/InteractiveLaunchSpecService.cs index e0f65201..f927828d 100644 --- a/src/ClaudeDo.Worker/Runner/InteractiveLaunchSpecService.cs +++ b/src/ClaudeDo.Worker/Runner/InteractiveLaunchSpecService.cs @@ -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 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; } /// 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) diff --git a/tests/ClaudeDo.Worker.Tests/External/HandoffMcpToolsTests.cs b/tests/ClaudeDo.Worker.Tests/External/HandoffMcpToolsTests.cs index f7f6450d..4767b5dd 100644 --- a/tests/ClaudeDo.Worker.Tests/External/HandoffMcpToolsTests.cs +++ b/tests/ClaudeDo.Worker.Tests/External/HandoffMcpToolsTests.cs @@ -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() { diff --git a/tests/ClaudeDo.Worker.Tests/Hub/MergeHelperTaskHubTests.cs b/tests/ClaudeDo.Worker.Tests/Hub/MergeHelperTaskHubTests.cs index ed3535c3..91b919d9 100644 --- a/tests/ClaudeDo.Worker.Tests/Hub/MergeHelperTaskHubTests.cs +++ b/tests/ClaudeDo.Worker.Tests/Hub/MergeHelperTaskHubTests.cs @@ -64,7 +64,7 @@ public sealed class MergeHelperTaskHubTests : IDisposable var wtManager = new WorktreeManager(git, factory, new WorkerConfig(), NullLogger.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); diff --git a/tests/ClaudeDo.Worker.Tests/Runner/InteractiveLaunchSpecServiceTests.cs b/tests/ClaudeDo.Worker.Tests/Runner/InteractiveLaunchSpecServiceTests.cs index cf433340..cc2a3e5a 100644 --- a/tests/ClaudeDo.Worker.Tests/Runner/InteractiveLaunchSpecServiceTests.cs +++ b/tests/ClaudeDo.Worker.Tests/Runner/InteractiveLaunchSpecServiceTests.cs @@ -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>(Installed); } - private InteractiveLaunchSpecService BuildService() => + private InteractiveLaunchSpecService BuildService(HandoffRoundTracker? handoffRounds = null) => new(_db.CreateFactory(), _seeder, _registry, new WorktreeManager(new GitService(), _db.CreateFactory(), new WorkerConfig(), NullLogger.Instance), new GitService(), - new WorkerConfig { ClaudeBin = _claudeStubPath }); + new WorkerConfig { ClaudeBin = _claudeStubPath }, + handoffRounds ?? new HandoffRoundTracker()); private async Task 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);