From 016b9203f8376540c0ff75dc1934d1a2bb4f8d09 Mon Sep 17 00:00:00 2001 From: mika kuns Date: Wed, 2 Sep 2026 19:17:41 +0200 Subject: [PATCH] feat(worker): Handler-Session nach Usage-Limit-Reset automatisch neu starten Eine List-Handler-ConPTY-Session, die mitten in einer Phase stirbt, war bisher ein stilles totes Tile: InteractiveTerminalViewModel.OnSessionProcessExited setzte nur bei einem Tod innerhalb des 5s-Grace-Fensters ein sichtbares Banner. Neu: - HandlerRoundState (JSON unter ~/.claudeDo/handler-rounds.json, keine EF-Migration): Phase + Task-Set der laufenden Runde, geschrieben von BuildForMergeHelperAsync ("triage") und BuildForMergeHelperHandoffAsync. Ohne das existieren beide nur im transienten HandoffRequested-Broadcast. - HandlerRestartCoordinator: entscheidet den Neustart, fail-closed. Beleg fuer "war das Usage-Limit" ist FiveHour.Utilization >= 95 zum Todeszeitpunkt -- eine ConPTY-Session hat kein stdout, TaskRunner.IsUsageLimitMessage hat hier nichts zu lesen. Nur bei PowerUse (beide Keys), max ein Neustart pro Runde, Warten bis FiveHour.ResetsAt, dann HandoffRequested erneut broadcasten. Verbraucht kein Handoff-Budget (HandoffRoundTracker.Register wird nicht angefasst). WorkerLog-Zeilen beim Tod und beim Neustart. - Ui meldet den Exit-Code (nur dort bekannt) via WorkerHub.ReportInteractiveSessionExited, ohne selbst zu filtern. Sibling zu UsageLimitAutoContinueCoordinator, kein Umbau davon: der sieht nur headless Runs. --- docs/explore-notes/conpty-sessions.md | 37 +++ .../Services/Interfaces/IWorkerClient.cs | 4 + src/ClaudeDo.Ui/Services/WorkerClient.cs | 3 + .../InteractiveTerminalViewModel.cs | 14 +- .../MissionControl/ConPtyPaneViewModel.cs | 44 +++- .../ViewModels/MissionControlViewModel.cs | 20 ++ src/ClaudeDo.Worker/Hub/WorkerHub.cs | 17 +- src/ClaudeDo.Worker/Program.cs | 2 + .../Runner/HandlerRestartCoordinator.cs | 129 +++++++++ .../Runner/HandlerRoundState.cs | 138 ++++++++++ .../Runner/InteractiveLaunchSpecService.cs | 21 +- tests/ClaudeDo.Ui.Tests/StubWorkerClient.cs | 2 + .../ConPtyPaneViewModelTests.cs | 58 +++++ .../InteractiveTerminalViewModelTests.cs | 50 ++++ .../MissionControlViewModelTests.cs | 55 ++++ .../Hub/MergeHelperTaskHubTests.cs | 4 +- .../Runner/HandlerRestartCoordinatorTests.cs | 246 ++++++++++++++++++ .../InteractiveLaunchSpecServiceTests.cs | 11 +- 18 files changed, 843 insertions(+), 12 deletions(-) create mode 100644 src/ClaudeDo.Worker/Runner/HandlerRestartCoordinator.cs create mode 100644 src/ClaudeDo.Worker/Runner/HandlerRoundState.cs create mode 100644 tests/ClaudeDo.Worker.Tests/Runner/HandlerRestartCoordinatorTests.cs diff --git a/docs/explore-notes/conpty-sessions.md b/docs/explore-notes/conpty-sessions.md index e4251c70..d88df85e 100644 --- a/docs/explore-notes/conpty-sessions.md +++ b/docs/explore-notes/conpty-sessions.md @@ -143,6 +143,43 @@ outgoing session is only ever told to end its turn, never to exit. This keeps th `MCP_TOOL_TIMEOUT` is 200 s here — `TaskWaitMcpTools` clamps its own timeout to 170 s to stay comfortably under it (see [external-mcp.md](external-mcp.md)). +### Auto-restart after the usage limit + +A handler session that dies mid-phase used to be a silent dead tile: `InteractiveTerminalViewModel. +OnSessionProcessExited` only raised a visible banner for a death **inside** its 5 s +`StartupGraceWindow`. It now raises `ExitedAfterStartup` for a later non-zero exit; the pane +forwards it as `SessionDiedAfterStartup(taskId, exitCode)` and `MissionControlViewModel` reports it +via `WorkerHub.ReportInteractiveSessionExited`. Every task-based pane reports — the UI does no +filtering, because the exit code is the *only* thing it knows. + +`HandlerRestartCoordinator` (Worker) decides the rest and **fails closed** — unlike the usage gate, +every missing signal means "don't relaunch": + +- **Which round was running** comes from `HandlerRoundState`, a JSON file at + `~/.claudeDo/handler-rounds.json` (one `HandlerRound` per handler task id, deliberately not a DB + column). `BuildForMergeHelperAsync` writes the opening round as phase `"triage"`; + `BuildForMergeHelperHandoffAsync` writes each subsequent `MergeHelperPhase`. Without it the phase + and task set exist *only* in the transient `HandoffRequested` broadcast, so a dead tile could not + be reopened. A `"triage"` round is never reopened — there is no handoff spec for it. +- **Was it really the usage limit?** A ConPTY session has no captured stdout, so `TaskRunner. + IsUsageLimitMessage` (which matches the CLI's own "session limit" text) has nothing to read. The + corroborating witness is `UsageState.Snapshot.FiveHour.Utilization ≥ 95` + (`HandlerRestartCoordinator.UsageLimitProofPct`) at the moment of death. A crash at 40 % is not + restarted; no snapshot at all is not restarted. +- **PowerUse only** — both keys, exactly like the prompt addendum above. +- **One restart per round**, claimed via `HandlerRoundState.TryClaimAutoRestart` before the wait, so + a restart that dies again stays dead. `HandlerRoundState.Save` carries `AutoRestartUsed` over when + the phase is unchanged, because the restart's own launch-spec build writes the round again — a + naive overwrite would hand out a fresh budget every time and loop all night. +- The restart waits out `FiveHour.ResetsAt`, then re-broadcasts `HandoffRequested` with the + persisted phase + task set, which the UI already handles by reopening the tile. It goes **around** + `handoff_list_handler`, so `HandoffRoundTracker.Register` is never called and the run loses no + handoff budget. + +Sibling to `UsageLimitAutoContinueCoordinator`, not an extension of it: that one only ever sees +headless runs (`FailureReason == "usage_limit"`, resumed through the override slot), and a ConPTY +session has no `task_runs` row to classify. + ### The host task and its commit range The handler run **owns a real ClaudeDo task**, created by `CreateMergeHelperTask` (hub) → diff --git a/src/ClaudeDo.Ui/Services/Interfaces/IWorkerClient.cs b/src/ClaudeDo.Ui/Services/Interfaces/IWorkerClient.cs index 44be700f..e952a3c3 100644 --- a/src/ClaudeDo.Ui/Services/Interfaces/IWorkerClient.cs +++ b/src/ClaudeDo.Ui/Services/Interfaces/IWorkerClient.cs @@ -146,6 +146,10 @@ public interface IWorkerClient : INotifyPropertyChanged /// session's role: "wait" | "merge" | "wait_final" | "merge_final". Task GetMergeHelperHandoffLaunchSpecAsync( string taskId, IReadOnlyList survivingTaskIds, string nextPhase, CancellationToken ct = default); + /// Reports that a task-based ConPTY session's process exited non-zero after it had been + /// running. The exit code only exists in this process; the worker decides whether it was a + /// list-handler session killed by the usage limit and reopens the same phase after the 5h reset. + Task ReportInteractiveSessionExitedAsync(string taskId, int exitCode, CancellationToken ct = default); /// Starts a planning session and returns the launch spec for an embedded ConPTY /// planning terminal (replaces StartPlanningSessionAsync's external wt window). Task GetPlanningStartLaunchSpecAsync(string taskId, CancellationToken ct = default); diff --git a/src/ClaudeDo.Ui/Services/WorkerClient.cs b/src/ClaudeDo.Ui/Services/WorkerClient.cs index af0cadcc..f97140e5 100644 --- a/src/ClaudeDo.Ui/Services/WorkerClient.cs +++ b/src/ClaudeDo.Ui/Services/WorkerClient.cs @@ -626,6 +626,9 @@ public partial class WorkerClient : ObservableObject, IAsyncDisposable, IWorkerC string taskId, IReadOnlyList survivingTaskIds, string nextPhase, CancellationToken ct = default) => await InvokeTimedAsync("GetMergeHelperHandoffLaunchSpec", () => _hub.InvokeAsync("GetMergeHelperHandoffLaunchSpec", taskId, survivingTaskIds, nextPhase, ct)); + public Task ReportInteractiveSessionExitedAsync(string taskId, int exitCode, CancellationToken ct = default) + => InvokeTimedAsync("ReportInteractiveSessionExited", () => _hub.InvokeAsync("ReportInteractiveSessionExited", taskId, exitCode, ct)); + public async Task GetPlanningStartLaunchSpecAsync(string taskId, CancellationToken ct = default) => await InvokeTimedAsync("GetPlanningStartLaunchSpec", () => _hub.InvokeAsync("GetPlanningStartLaunchSpec", taskId, ct)); diff --git a/src/ClaudeDo.Ui/ViewModels/InteractiveTerminalViewModel.cs b/src/ClaudeDo.Ui/ViewModels/InteractiveTerminalViewModel.cs index 94770137..3468aa2d 100644 --- a/src/ClaudeDo.Ui/ViewModels/InteractiveTerminalViewModel.cs +++ b/src/ClaudeDo.Ui/ViewModels/InteractiveTerminalViewModel.cs @@ -96,6 +96,13 @@ public sealed partial class InteractiveTerminalViewModel : ViewModelBase, IDispo } } + /// Raised when the process died non-zero *after* clearing — + /// i.e. a session that ran, then broke. That used to be a silent dead tile with no signal at all; + /// the worker turns it into a usage-limit auto-restart for list-handler sessions + /// (HandlerRestartCoordinator). A startup death keeps using instead: + /// it never reached a state worth resuming. + public event Action? ExitedAfterStartup; + /// Internal (not private) so tests can drive it directly without a real ConPTY spawn. internal void OnSessionProcessExited(object? sender, int exitCode) { @@ -103,11 +110,16 @@ public sealed partial class InteractiveTerminalViewModel : ViewModelBase, IDispo HasExited = true; ExitCode = exitCode; + if (exitCode == 0) return; + if (_startedAtUtc is not { } startedAt) return; // never reached "running" — nothing ran to resume + // Died at startup: route through the same banner as a launch-time failure instead of // leaving a dead terminal with no Retry affordance (ConPtyPaneViewModel.CanRetry // requires StartError). - if (exitCode != 0 && _startedAtUtc is { } startedAt && _utcNow() - startedAt < StartupGraceWindow) + if (_utcNow() - startedAt < StartupGraceWindow) StartError = $"Session exited immediately (code {exitCode})."; + else + ExitedAfterStartup?.Invoke(exitCode); } /// Reports a failure that happened before could be called (e.g. the diff --git a/src/ClaudeDo.Ui/ViewModels/MissionControl/ConPtyPaneViewModel.cs b/src/ClaudeDo.Ui/ViewModels/MissionControl/ConPtyPaneViewModel.cs index 6fd7bf82..8fe2455d 100644 --- a/src/ClaudeDo.Ui/ViewModels/MissionControl/ConPtyPaneViewModel.cs +++ b/src/ClaudeDo.Ui/ViewModels/MissionControl/ConPtyPaneViewModel.cs @@ -36,6 +36,12 @@ public sealed partial class ConPtyPaneViewModel : ViewModelBase, IDisposable /// commits the worktree and moves the task to WaitingForReview. public event Action? SubmitForReviewRequested; + /// Raised with (taskId, exitCode) when a task-based session that had been running + /// dies non-zero. The host reports it to the worker, which decides whether this was a list + /// handler killed by the usage limit and worth reopening. Ad-hoc panes never raise it — there is + /// no task to reopen. + public event Action? SessionDiedAfterStartup; + /// Task-based pane — dedup'd by . Pass null for an ad-hoc pane /// (no task, never deduped); prefer at ad-hoc call sites. /// is resolved by the pane itself (in ) @@ -49,7 +55,34 @@ public sealed partial class ConPtyPaneViewModel : ViewModelBase, IDisposable TaskId = taskId; _displayTitle = displayTitle; _descriptorFactory = descriptorFactory; - Terminal.PropertyChanged += OnTerminalPropertyChanged; + Subscribe(Terminal); + } + + // Terminal is swapped wholesale on Retry, so the handlers follow the property rather than being + // wired once at construction. (The initial value comes from the field initializer, which never + // runs through the setter -- hence the explicit Subscribe in the constructor.) + partial void OnTerminalChanging(InteractiveTerminalViewModel? oldValue, InteractiveTerminalViewModel newValue) + { + if (oldValue is not null) Unsubscribe(oldValue); + } + + partial void OnTerminalChanged(InteractiveTerminalViewModel value) => Subscribe(value); + + private void Subscribe(InteractiveTerminalViewModel terminal) + { + terminal.PropertyChanged += OnTerminalPropertyChanged; + terminal.ExitedAfterStartup += OnTerminalExitedAfterStartup; + } + + private void Unsubscribe(InteractiveTerminalViewModel terminal) + { + terminal.PropertyChanged -= OnTerminalPropertyChanged; + terminal.ExitedAfterStartup -= OnTerminalExitedAfterStartup; + } + + private void OnTerminalExitedAfterStartup(int exitCode) + { + if (TaskId is { } id) SessionDiedAfterStartup?.Invoke(id, exitCode); } private readonly Func> _descriptorFactory; @@ -111,16 +144,15 @@ public sealed partial class ConPtyPaneViewModel : ViewModelBase, IDisposable [RelayCommand(CanExecute = nameof(CanRetry))] private void Retry() { - Terminal.PropertyChanged -= OnTerminalPropertyChanged; - Terminal.Dispose(); - Terminal = new InteractiveTerminalViewModel(); - Terminal.PropertyChanged += OnTerminalPropertyChanged; + var dead = Terminal; + Terminal = new InteractiveTerminalViewModel(); // OnTerminalChanging/Changed move the handlers + dead.Dispose(); Start(); } public void Dispose() { - Terminal.PropertyChanged -= OnTerminalPropertyChanged; + Unsubscribe(Terminal); Terminal.Kill(); Terminal.Dispose(); } diff --git a/src/ClaudeDo.Ui/ViewModels/MissionControlViewModel.cs b/src/ClaudeDo.Ui/ViewModels/MissionControlViewModel.cs index ab7d1ffd..45c210b1 100644 --- a/src/ClaudeDo.Ui/ViewModels/MissionControlViewModel.cs +++ b/src/ClaudeDo.Ui/ViewModels/MissionControlViewModel.cs @@ -413,10 +413,28 @@ public sealed partial class MissionControlViewModel : ViewModelBase, IDisposable pane.ErrorReported += OnConPtyPaneError; pane.CloseRequested += CloseConPtySession; pane.SubmitForReviewRequested += OnPaneSubmitForReview; + pane.SessionDiedAfterStartup += OnPaneSessionDied; ConPtySessions.Add(pane); pane.Start(); } + // A ConPTY session that had been running died. The UI process owns the terminal, so this is the + // only place the exit code exists -- report it and let the worker decide whether it was a list + // handler killed by the usage limit and worth reopening after the reset + // (HandlerRestartCoordinator). Every other pane is just a no-op there, so no filtering here. + private async void OnPaneSessionDied(string taskId, int exitCode) + { + try + { + await _worker.ReportInteractiveSessionExitedAsync(taskId, exitCode); + } + catch (Exception ex) + { + // A worker that is down cannot restart anything anyway; the dead tile is already visible. + ErrorReported?.Invoke(Loc.T("missionControl.conptyLaunchFailed", ex.Message)); + } + } + private static async System.Threading.Tasks.Task DescribeAsync( Func> fetch) { @@ -457,6 +475,7 @@ public sealed partial class MissionControlViewModel : ViewModelBase, IDisposable pane.ErrorReported -= OnConPtyPaneError; pane.CloseRequested -= CloseConPtySession; pane.SubmitForReviewRequested -= OnPaneSubmitForReview; + pane.SessionDiedAfterStartup -= OnPaneSessionDied; ConPtySessions.Remove(pane); pane.Dispose(); } @@ -507,6 +526,7 @@ public sealed partial class MissionControlViewModel : ViewModelBase, IDisposable c.ErrorReported -= OnConPtyPaneError; c.CloseRequested -= CloseConPtySession; c.SubmitForReviewRequested -= OnPaneSubmitForReview; + c.SessionDiedAfterStartup -= OnPaneSessionDied; c.Dispose(); } ConPtySessions.Clear(); diff --git a/src/ClaudeDo.Worker/Hub/WorkerHub.cs b/src/ClaudeDo.Worker/Hub/WorkerHub.cs index 6db12bd9..8f088d6d 100644 --- a/src/ClaudeDo.Worker/Hub/WorkerHub.cs +++ b/src/ClaudeDo.Worker/Hub/WorkerHub.cs @@ -72,6 +72,7 @@ public sealed class WorkerHub : Microsoft.AspNetCore.SignalR.Hub private readonly TicketPatStore _ticketPat; private readonly TicketClientFactory _ticketClients; private readonly TicketImportService _ticketImport; + private readonly HandlerRestartCoordinator? _handlerRestart; public WorkerHub( QueueService queue, @@ -110,7 +111,8 @@ public sealed class WorkerHub : Microsoft.AspNetCore.SignalR.Hub TicketSystemConfig ticketConfig = null!, TicketPatStore ticketPat = null!, TicketClientFactory ticketClients = null!, - TicketImportService ticketImport = null!) + TicketImportService ticketImport = null!, + HandlerRestartCoordinator? handlerRestart = null) { _queue = queue; _waker = waker; @@ -149,6 +151,7 @@ public sealed class WorkerHub : Microsoft.AspNetCore.SignalR.Hub _ticketPat = ticketPat; _ticketClients = ticketClients; _ticketImport = ticketImport; + _handlerRestart = handlerRestart; } // Persistence boundary for the session_skills JSON-array columns (task/list/global). @@ -786,6 +789,18 @@ public sealed class WorkerHub : Microsoft.AspNetCore.SignalR.Hub return _interactiveLaunchSpec.BuildForMergeHelperHandoffAsync(taskId, survivingTaskIds, nextPhase, Context.ConnectionAborted); }); + // Reports that an embedded ConPTY session's process exited. Only the UI process owns the + // terminal, so the exit code exists nowhere else; every decision about it (was this a list + // handler, did the usage limit kill it, is PowerUse on, when does the window reset) belongs to + // HandlerRestartCoordinator. Fire-and-forget on purpose: the coordinator's task stays parked + // until the 5h reset, which is far longer than this connection or this hub instance lives, so + // it must not run under Context.ConnectionAborted either. + public void ReportInteractiveSessionExited(string taskId, int exitCode) + { + if (_handlerRestart is null) return; + _ = _handlerRestart.OnSessionExitedAsync(taskId, exitCode, CancellationToken.None); + } + // Starts a planning session (worktree + prompt files + token, task -> Planning) and returns // the launch spec for an embedded ConPTY planning terminal -- the ConPTY replacement for // StartPlanningSessionAsync's external wt window. On any spec-build failure the just-started diff --git a/src/ClaudeDo.Worker/Program.cs b/src/ClaudeDo.Worker/Program.cs index 36d68698..5ef1b631 100644 --- a/src/ClaudeDo.Worker/Program.cs +++ b/src/ClaudeDo.Worker/Program.cs @@ -95,6 +95,8 @@ builder.Services.AddSingleton(); builder.Services.AddSingleton(); builder.Services.AddSingleton(); builder.Services.AddSingleton(); +builder.Services.AddSingleton(); +builder.Services.AddSingleton(); builder.Services.AddSingleton(); builder.Services.AddSingleton(); builder.Services.AddSingleton(); diff --git a/src/ClaudeDo.Worker/Runner/HandlerRestartCoordinator.cs b/src/ClaudeDo.Worker/Runner/HandlerRestartCoordinator.cs new file mode 100644 index 00000000..dca9c6d9 --- /dev/null +++ b/src/ClaudeDo.Worker/Runner/HandlerRestartCoordinator.cs @@ -0,0 +1,129 @@ +using ClaudeDo.Data; +using ClaudeDo.Data.Models; +using ClaudeDo.Data.Repositories; +using ClaudeDo.Worker.Hub; +using ClaudeDo.Worker.Usage; +using Microsoft.EntityFrameworkCore; + +namespace ClaudeDo.Worker.Runner; + +/// +/// Reopens a list-handler ConPTY session that died on the Anthropic usage limit, once the 5h window +/// resets — the interactive counterpart to UsageLimitAutoContinueCoordinator, which only ever +/// sees headless task runs. Deliberately a sibling and not an extension of it: a ConPTY session has +/// no task_runs row, no FailureReason, and is resumed by relaunching a phase rather +/// than by ContinueTask through the override slot. +/// +/// Driven by the UI (WorkerHub.ReportInteractiveSessionExited) because the exit code only +/// exists in the process that owns the terminal. Everything else — was it the usage limit, is +/// PowerUse on, which phase and task set, when does the window reset — is decided here. +/// +/// Fails CLOSED, unlike the usage gate: every missing signal means "don't relaunch". An interactive +/// session that pops back up at 3am uninvited is worse than one that stays dead. +/// +public sealed class HandlerRestartCoordinator +{ + // The corroborating signal that this exit really was the usage limit. A ConPTY session gives us + // nothing but an exit code -- there is no stdout to match against, so TaskRunner's + // IsUsageLimitMessage regex (which reads the CLI's own "session limit reached" text) has nothing + // to read here. The 5h utilization at the moment of death is the one independent witness we do + // have, and it is pinned at 100% when the limit is what stopped the session. A crash, a manual + // Ctrl-C, or a bad launch at 40% utilization is therefore NOT restarted. + internal const int UsageLimitProofPct = 95; + + private readonly IDbContextFactory _dbFactory; + private readonly UsageState _usageState; + private readonly HandlerRoundState _rounds; + private readonly HubBroadcaster _broadcaster; + private readonly ILogger _logger; + + public HandlerRestartCoordinator( + IDbContextFactory dbFactory, + UsageState usageState, + HandlerRoundState rounds, + HubBroadcaster broadcaster, + ILogger logger) + { + _dbFactory = dbFactory; + _usageState = usageState; + _rounds = rounds; + _broadcaster = broadcaster; + _logger = logger; + } + + /// Test seams — a real 5h wait is not a test. + internal Func NowUtc { get; set; } = () => DateTimeOffset.UtcNow; + internal Func Delay { get; set; } = (d, ct) => Task.Delay(d, ct); + + /// Handles a ConPTY session's process exit. Returns true when a restart was claimed and + /// carried out (the returned task completes only after the wait, so a caller that wants + /// fire-and-forget must not await it). Any non-handler task, clean exit, non-usage-limit death, + /// PowerUse-off list, or already-used round is a silent no-op. + public async Task OnSessionExitedAsync(string taskId, int exitCode, CancellationToken ct = default) + { + if (exitCode == 0 || string.IsNullOrEmpty(taskId)) return false; + + var round = _rounds.Get(taskId); + // Not a list-handler session at all (a plain interactive task pane), or the initial triage + // session, which has no handoff launch spec to reopen -- BuildForMergeHelperAsync needs the + // list id and the original selection, and its dedupe/enhance work is not idempotent. + if (round is null || !MergeHelperPhase.All.Contains(round.Phase, StringComparer.Ordinal)) return false; + + var snapshot = _usageState.Snapshot; + if (snapshot?.FiveHour is not { } fiveHour) return false; + if (fiveHour.Utilization < UsageLimitProofPct) return false; + if (fiveHour.ResetsAt is not { } resetsAt) return false; + + int number; + try + { + await using var ctx = await _dbFactory.CreateDbContextAsync(ct); + var task = await new TaskRepository(ctx).GetByIdAsync(taskId, ct); + if (task is null) return false; + number = task.Number; + + // Both PowerUse keys, exactly as ComposeHandlerPromptAsync gates the PowerUse prompt + // addendum: the global switch alone must not make an interactive run on some unrelated + // list restart itself overnight. + if (!(await new AppSettingsRepository(ctx).GetAsync(ct)).PowerUseEnabled) return false; + if ((await new ListRepository(ctx).GetConfigAsync(task.ListId, ct))?.PowerUseAllowed != true) return false; + } + catch (Exception ex) + { + _logger.LogWarning(ex, "HandlerRestartCoordinator: could not evaluate handler task {TaskId}; not restarting", taskId); + return false; + } + + // One restart per round, claimed before the wait -- a restart that dies again lands back + // here and must stop, not relaunch. + if (!_rounds.TryClaimAutoRestart(taskId)) return false; + + try + { + await _broadcaster.WorkerLog( + $"List handler #{number} died at the usage limit (5h {fiveHour.Utilization:0}%) — " + + $"restarting phase {round.Phase} after the reset at {resetsAt.ToLocalTime():HH:mm}", + WorkerLogLevel.Warn, DateTime.UtcNow); + + var wait = resetsAt - NowUtc(); + if (wait > TimeSpan.Zero) await Delay(wait, ct); + + await _broadcaster.HandoffRequested(taskId, round.TaskIds, round.Phase); + await _broadcaster.WorkerLog( + $"Auto-restarting list handler #{number} at phase {round.Phase} after usage-limit reset", + WorkerLogLevel.Info, DateTime.UtcNow); + return true; + } + catch (OperationCanceledException) + { + return false; + } + catch (Exception ex) + { + // Callers fire-and-forget this (WorkerHub.ReportInteractiveSessionExited), so an + // unobserved throw here would be invisible. + _logger.LogError(ex, "HandlerRestartCoordinator: auto-restart of handler task {TaskId} failed", taskId); + return false; + } + } +} diff --git a/src/ClaudeDo.Worker/Runner/HandlerRoundState.cs b/src/ClaudeDo.Worker/Runner/HandlerRoundState.cs new file mode 100644 index 00000000..55e261bb --- /dev/null +++ b/src/ClaudeDo.Worker/Runner/HandlerRoundState.cs @@ -0,0 +1,138 @@ +using System.Text.Json; +using ClaudeDo.Data; + +namespace ClaudeDo.Worker.Runner; + +/// One list-handler round: the phase a session is running and the exact task set it was +/// handed. Written every time a handler session is built, so an outside observer (a crashed tile, +/// a deferred handoff) can reopen the SAME phase with the SAME set without the in-flight +/// HandoffRequested broadcast that is the only other place these ids exist. +/// A value, or "triage" for the initial session +/// (which has no handoff spec and therefore cannot be reopened — see HandlerRestartCoordinator). +/// Claimed once per round by HandlerRestartCoordinator, so a handler +/// that dies again after its auto-restart stays dead instead of relaunching all night. +/// Reserved for the "hand off cleanly before the usage limit" +/// follow-up, which parks a handoff until the 5h reset and needs to survive a worker restart. +public sealed record HandlerRound( + string HandlerTaskId, + string Phase, + IReadOnlyList TaskIds, + DateTime StartedAtUtc, + bool AutoRestartUsed = false, + DateTimeOffset? DeferredHandoffAt = null); + +/// +/// File-backed store for , one entry per handler task. Deliberately a +/// JSON file under ~/.claudeDo/ (same Load/Save shape as ClaudeDo.Ui.AppSettings) +/// rather than a DB column: rounds are process-lifetime scratch state that no query joins against, +/// and an EF migration here would collide with the one the settings work owns this round. +/// Every read/write goes through one lock and re-serializes the whole file — there are at most a +/// handful of concurrent handler runs, so nothing here needs to be clever. +/// +public sealed class HandlerRoundState +{ + // Entries older than this are dropped on the next save. A round that has been open for a week + // is not coming back, and without a sweep the file only ever grows. + private static readonly TimeSpan Retention = TimeSpan.FromDays(7); + + private readonly string _path; + private readonly object _lock = new(); + private Dictionary? _rounds; + + public HandlerRoundState() : this(Path.Combine(Paths.AppDataRoot(), "handler-rounds.json")) + { + } + + /// Test seam — point the store at a temp file instead of the live app data root. + internal HandlerRoundState(string path) => _path = path; + + public HandlerRound? Get(string handlerTaskId) + { + lock (_lock) + return Rounds().GetValueOrDefault(handlerTaskId); + } + + /// Overwrites this handler task's round. is + /// carried over when the phase is unchanged: the auto-restart itself rebuilds the same phase's + /// launch spec, which lands right back here, and a naive overwrite would hand the round a fresh + /// restart budget every time — exactly the all-night relaunch loop the claim exists to prevent. + /// A genuinely new phase is a new round and starts with a fresh budget. + public void Save(HandlerRound round) + { + lock (_lock) + { + var rounds = Rounds(); + if (rounds.TryGetValue(round.HandlerTaskId, out var previous) + && string.Equals(previous.Phase, round.Phase, StringComparison.Ordinal) + && previous.AutoRestartUsed) + { + round = round with { AutoRestartUsed = true }; + } + + rounds[round.HandlerTaskId] = round; + Persist(rounds); + } + } + + /// Takes this round's single auto-restart budget. False when there is no round for the + /// task or its budget is already spent — the same "never twice" discipline + /// TaskRepository.TryClaimUsageLimitAutoContinueAsync enforces for headless runs. + public bool TryClaimAutoRestart(string handlerTaskId) + { + lock (_lock) + { + var rounds = Rounds(); + if (!rounds.TryGetValue(handlerTaskId, out var round) || round.AutoRestartUsed) return false; + + rounds[handlerTaskId] = round with { AutoRestartUsed = true }; + Persist(rounds); + return true; + } + } + + public void Remove(string handlerTaskId) + { + lock (_lock) + { + var rounds = Rounds(); + if (rounds.Remove(handlerTaskId)) Persist(rounds); + } + } + + // Callers hold _lock. + private Dictionary Rounds() + { + if (_rounds is not null) return _rounds; + try + { + if (File.Exists(_path)) + { + _rounds = JsonSerializer.Deserialize>( + File.ReadAllText(_path), new JsonSerializerOptions { PropertyNameCaseInsensitive = true }); + } + } + catch + { + // A corrupt/unreadable file must never block a handler launch — start empty. + } + return _rounds ??= new Dictionary(StringComparer.Ordinal); + } + + private void Persist(Dictionary rounds) + { + var cutoff = DateTime.UtcNow - Retention; + foreach (var stale in rounds.Where(kv => kv.Value.StartedAtUtc < cutoff).Select(kv => kv.Key).ToList()) + rounds.Remove(stale); + + try + { + var dir = Path.GetDirectoryName(_path); + if (!string.IsNullOrEmpty(dir)) Directory.CreateDirectory(dir); + File.WriteAllText(_path, JsonSerializer.Serialize(rounds, new JsonSerializerOptions { WriteIndented = true })); + } + catch + { + // Losing persistence costs an auto-restart across a worker restart, never a launch. + } + } +} diff --git a/src/ClaudeDo.Worker/Runner/InteractiveLaunchSpecService.cs b/src/ClaudeDo.Worker/Runner/InteractiveLaunchSpecService.cs index d5e041bc..f15686bc 100644 --- a/src/ClaudeDo.Worker/Runner/InteractiveLaunchSpecService.cs +++ b/src/ClaudeDo.Worker/Runner/InteractiveLaunchSpecService.cs @@ -44,6 +44,7 @@ public sealed class InteractiveLaunchSpecService private readonly GitService _git; private readonly string _claudePath; private readonly HandoffRoundTracker _handoffRounds; + private readonly HandlerRoundState _roundState; public InteractiveLaunchSpecService( IDbContextFactory dbFactory, @@ -52,7 +53,8 @@ public sealed class InteractiveLaunchSpecService WorktreeManager wtManager, GitService git, WorkerConfig cfg, - HandoffRoundTracker handoffRounds) + HandoffRoundTracker handoffRounds, + HandlerRoundState roundState) { _dbFactory = dbFactory; _skillSeeder = skillSeeder; @@ -61,6 +63,7 @@ public sealed class InteractiveLaunchSpecService _git = git; _claudePath = cfg.ClaudeBin; _handoffRounds = handoffRounds; + _roundState = roundState; } /// Builds a LaunchSpec for opening an interactive session in a task's worktree. @@ -342,9 +345,20 @@ public sealed class InteractiveLaunchSpecService ["MCP_TOOL_TIMEOUT"] = McpToolTimeoutMs, }; + // The opening round. Recorded so anything watching a handler run knows which round is live + // (the deferred-handoff follow-up needs that), but marked "triage" rather than a + // MergeHelperPhase value on purpose: there is no handoff spec for this phase, so + // HandlerRestartCoordinator will not reopen it. + _roundState.Save(new HandlerRound(handlerTaskId, TriagePhase, taskIds, DateTime.UtcNow)); + return new LaunchSpec(repoDir, resolvedClaude, args, env); } + /// The marker for the initial (dedupe/enhance) handler + /// session. Deliberately not a value — that set is exactly the + /// phases can rebuild. + public const string TriagePhase = "triage"; + // Builds the LaunchSpec for the fresh session a merge-helper run hands off to for the given // phase -- SAME handler task id as the run that called handoff_list_handler, so // HandlerBaseCommit/HandlerHeadCommit and the review range stay untouched; this never creates @@ -449,6 +463,11 @@ public sealed class InteractiveLaunchSpecService ["MCP_TOOL_TIMEOUT"] = McpToolTimeoutMs, }; + // Persist the round that is about to start. Phase + task set only exist in the transient + // HandoffRequested broadcast otherwise, so without this a dead tile cannot be reopened at + // the same phase with the same set (see HandlerRestartCoordinator). + _roundState.Save(new HandlerRound(taskId, nextPhase, survivingTaskIds, DateTime.UtcNow)); + return new LaunchSpec(repoDir, resolvedClaude, args, env); } diff --git a/tests/ClaudeDo.Ui.Tests/StubWorkerClient.cs b/tests/ClaudeDo.Ui.Tests/StubWorkerClient.cs index 5e7a2e35..382ac582 100644 --- a/tests/ClaudeDo.Ui.Tests/StubWorkerClient.cs +++ b/tests/ClaudeDo.Ui.Tests/StubWorkerClient.cs @@ -132,6 +132,8 @@ public abstract class StubWorkerClient : IWorkerClient public virtual Task GetMergeHelperHandoffLaunchSpecAsync( string taskId, IReadOnlyList survivingTaskIds, string nextPhase, CancellationToken ct = default) => Task.FromResult(new LaunchSpec(".", "claude", Array.Empty(), new Dictionary())); + public virtual Task ReportInteractiveSessionExitedAsync(string taskId, int exitCode, CancellationToken ct = default) + => Task.CompletedTask; public virtual Task GetPlanningStartLaunchSpecAsync(string taskId, CancellationToken ct = default) => Task.FromResult(new LaunchSpec(".", "claude", Array.Empty(), new Dictionary())); public virtual Task GetPlanningResumeLaunchSpecAsync(string taskId, CancellationToken ct = default) diff --git a/tests/ClaudeDo.Ui.Tests/ViewModels/MissionControl/ConPtyPaneViewModelTests.cs b/tests/ClaudeDo.Ui.Tests/ViewModels/MissionControl/ConPtyPaneViewModelTests.cs index b679b9d0..2b1d1a4a 100644 --- a/tests/ClaudeDo.Ui.Tests/ViewModels/MissionControl/ConPtyPaneViewModelTests.cs +++ b/tests/ClaudeDo.Ui.Tests/ViewModels/MissionControl/ConPtyPaneViewModelTests.cs @@ -1,4 +1,5 @@ using ClaudeDo.Ui.Services; +using ClaudeDo.Ui.ViewModels; using ClaudeDo.Ui.ViewModels.MissionControl; using Xunit; @@ -150,5 +151,62 @@ public class ConPtyPaneViewModelTests Assert.True(pane.RetryCommand.CanExecute(null)); } + // ── SessionDiedAfterStartup (feeds the worker's usage-limit auto-restart) ──────────────────── + + [Fact] + public void SessionDiedAfterStartup_TaskPane_RaisesWithTaskIdAndExitCode() + { + using var pane = NewTaskPane(NeverCompletes, taskId: "handler-1"); + (string TaskId, int ExitCode)? reported = null; + pane.SessionDiedAfterStartup += (id, code) => reported = (id, code); + + DieAfterRunning(pane, exitCode: 1); + + Assert.Equal(("handler-1", 1), reported); + } + + [Fact] + public void SessionDiedAfterStartup_AdHocPane_NeverRaises() + { + using var pane = ConPtyPaneViewModel.CreateAdHoc("Ad hoc", NeverCompletes); + var raised = false; + pane.SessionDiedAfterStartup += (_, _) => raised = true; + + DieAfterRunning(pane, exitCode: 1); + + Assert.False(raised); + } + + // Retry swaps in a fresh terminal -- the death of the RETRIED session must still be reported. + [Fact] + public void SessionDiedAfterStartup_StillWiredAfterRetry() + { + var callCount = 0; + using var pane = NewTaskPane( + () => callCount++ == 0 ? Failing() : NeverCompletes(), taskId: "handler-1"); + pane.Start(); + pane.RetryCommand.Execute(null); + + var raised = false; + pane.SessionDiedAfterStartup += (_, _) => raised = true; + + DieAfterRunning(pane, exitCode: 1); + + Assert.True(raised); + } + private static void SetRunning(ConPtyPaneViewModel pane) => pane.Terminal.IsRunning = true; + + // Drives the terminal past its startup grace window, then kills it: IsRunning stamps the launch + // time off the real clock, so a sleep-free "ran for a while" needs the exit routed through a + // terminal whose clock has already moved on. + private static void DieAfterRunning(ConPtyPaneViewModel pane, int exitCode) + { + var launchedAt = new DateTime(2026, 1, 1, 0, 0, 0, DateTimeKind.Utc); + var callCount = 0; + pane.Terminal = new InteractiveTerminalViewModel( + () => callCount++ == 0 ? launchedAt : launchedAt.AddMinutes(90)); + pane.Terminal.IsRunning = true; + pane.Terminal.OnSessionProcessExited(null, exitCode); + } } diff --git a/tests/ClaudeDo.Ui.Tests/ViewModels/MissionControl/InteractiveTerminalViewModelTests.cs b/tests/ClaudeDo.Ui.Tests/ViewModels/MissionControl/InteractiveTerminalViewModelTests.cs index 29eecc5c..4d6b7a5d 100644 --- a/tests/ClaudeDo.Ui.Tests/ViewModels/MissionControl/InteractiveTerminalViewModelTests.cs +++ b/tests/ClaudeDo.Ui.Tests/ViewModels/MissionControl/InteractiveTerminalViewModelTests.cs @@ -49,4 +49,54 @@ public class InteractiveTerminalViewModelTests Assert.True(terminal.HasExited); Assert.Null(terminal.StartError); } + + // ── ExitedAfterStartup (a session that ran, then broke -- used to be a silent dead tile; + // the worker turns it into a usage-limit auto-restart for list handlers) ─────────────────── + + [Fact] + public void ProcessExitsNonzero_AfterRunningAWhile_RaisesExitedAfterStartup() + { + using var terminal = NewTerminalExitingAfter(TimeSpan.FromMinutes(90)); + int? reported = null; + terminal.ExitedAfterStartup += code => reported = code; + + terminal.OnSessionProcessExited(null, 1); + + Assert.Equal(1, reported); + } + + [Fact] + public void ProcessExitsNonzero_WithinGraceWindow_DoesNotRaiseExitedAfterStartup() + { + using var terminal = new InteractiveTerminalViewModel(() => LaunchedAt); + terminal.IsRunning = true; + var raised = false; + terminal.ExitedAfterStartup += _ => raised = true; + + terminal.OnSessionProcessExited(null, 1); + + Assert.False(raised); + Assert.NotNull(terminal.StartError); // still the startup-failure banner + } + + [Fact] + public void ProcessExitsZero_AfterRunningAWhile_DoesNotRaiseExitedAfterStartup() + { + using var terminal = NewTerminalExitingAfter(TimeSpan.FromMinutes(90)); + var raised = false; + terminal.ExitedAfterStartup += _ => raised = true; + + terminal.OnSessionProcessExited(null, 0); + + Assert.False(raised); + } + + private static InteractiveTerminalViewModel NewTerminalExitingAfter(TimeSpan uptime) + { + var callCount = 0; + var terminal = new InteractiveTerminalViewModel( + () => callCount++ == 0 ? LaunchedAt : LaunchedAt + uptime); + terminal.IsRunning = true; // records LaunchedAt via the first clock call + return terminal; + } } diff --git a/tests/ClaudeDo.Ui.Tests/ViewModels/MissionControlViewModelTests.cs b/tests/ClaudeDo.Ui.Tests/ViewModels/MissionControlViewModelTests.cs index 5b395df3..557e1b29 100644 --- a/tests/ClaudeDo.Ui.Tests/ViewModels/MissionControlViewModelTests.cs +++ b/tests/ClaudeDo.Ui.Tests/ViewModels/MissionControlViewModelTests.cs @@ -867,6 +867,61 @@ public class MissionControlViewModelTests : IDisposable Assert.Equal(0, SubscriberCount(pane, "SubmitForReviewRequested")); } + // ── usage-limit auto-restart: the exit code has to reach the worker ───────────────────────── + + private sealed class RecordingExitWorker : StubWorkerClient + { + public readonly List<(string TaskId, int ExitCode)> Reported = new(); + public override Task ReportInteractiveSessionExitedAsync(string taskId, int exitCode, CancellationToken ct = default) + { + Reported.Add((taskId, exitCode)); + return Task.CompletedTask; + } + } + + // A handler tile that ran for a while and then died non-zero used to be a silent dead pane. + // Whether it was the usage limit (and worth reopening) is the worker's call, so the UI's only + // job is reporting the exit -- hence no filtering here, every task pane reports. + [Fact] + public async Task PaneSessionDiesAfterRunning_ReportsExitToWorker() + { + var worker = new RecordingExitWorker(); + using var vm = BuildVm(worker); + await vm.OpenMergeHelperHandoffConPtySessionAsync("handler-1", new[] { "t1" }, "merge"); + var pane = Assert.Single(vm.ConPtySessions); + + DieAfterRunning(pane, exitCode: 1); + + Assert.Equal(new[] { ("handler-1", 1) }, worker.Reported); + } + + [Fact] + public async Task ClosedPane_DoesNotReportItsSessionExit() + { + var worker = new RecordingExitWorker(); + using var vm = BuildVm(worker); + await vm.OpenMergeHelperHandoffConPtySessionAsync("handler-1", new[] { "t1" }, "merge"); + var pane = Assert.Single(vm.ConPtySessions); + + pane.CloseCommand.Execute(null); + DieAfterRunning(pane, exitCode: 1); + + Assert.Empty(worker.Reported); + } + + // Drives a pane's terminal past its startup grace window and then kills it. IsRunning stamps the + // launch time off the terminal's own clock, so a sleep-free "ran for 90 minutes" needs a fresh + // terminal with a stubbed clock -- the pane re-wires its handlers on assignment. + private static void DieAfterRunning(ConPtyPaneViewModel pane, int exitCode) + { + var launchedAt = new DateTime(2026, 1, 1, 0, 0, 0, DateTimeKind.Utc); + var callCount = 0; + pane.Terminal = new InteractiveTerminalViewModel( + () => callCount++ == 0 ? launchedAt : launchedAt.AddMinutes(90)); + pane.Terminal.IsRunning = true; + pane.Terminal.OnSessionProcessExited(null, exitCode); + } + [Fact] public void ToggleLayoutCommand_FlipsIsFocusMode() { diff --git a/tests/ClaudeDo.Worker.Tests/Hub/MergeHelperTaskHubTests.cs b/tests/ClaudeDo.Worker.Tests/Hub/MergeHelperTaskHubTests.cs index 91b919d9..c13472ee 100644 --- a/tests/ClaudeDo.Worker.Tests/Hub/MergeHelperTaskHubTests.cs +++ b/tests/ClaudeDo.Worker.Tests/Hub/MergeHelperTaskHubTests.cs @@ -64,7 +64,9 @@ 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 ClaudeDo.Worker.External.HandoffRoundTracker()); + new WorkerConfig { ClaudeBin = "claude" }, new ClaudeDo.Worker.External.HandoffRoundTracker(), + new ClaudeDo.Worker.Runner.HandlerRoundState( + Path.Combine(Path.GetTempPath(), $"claudedo_rounds_{Guid.NewGuid():N}.json"))); var built = TaskStateServiceBuilder.Build(factory); var reviewSubmission = new InteractiveReviewSubmissionService(factory, git, built.State, wtManager); diff --git a/tests/ClaudeDo.Worker.Tests/Runner/HandlerRestartCoordinatorTests.cs b/tests/ClaudeDo.Worker.Tests/Runner/HandlerRestartCoordinatorTests.cs new file mode 100644 index 00000000..72f5bec2 --- /dev/null +++ b/tests/ClaudeDo.Worker.Tests/Runner/HandlerRestartCoordinatorTests.cs @@ -0,0 +1,246 @@ +using ClaudeDo.Data.Models; +using ClaudeDo.Data.Repositories; +using ClaudeDo.Worker.External; +using ClaudeDo.Worker.Hub; +using ClaudeDo.Worker.Runner; +using ClaudeDo.Worker.Tests.Infrastructure; +using ClaudeDo.Worker.Usage; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.Logging.Abstractions; + +namespace ClaudeDo.Worker.Tests.Runner; + +/// Auto-restart of a list-handler ConPTY session that died at the usage limit. The UI half (the +/// exit code reaching the worker at all) lives in Ui.Tests' InteractiveTerminalViewModel / +/// MissionControlViewModel tests. +public sealed class HandlerRestartCoordinatorTests : IDisposable +{ + private readonly DbFixture _db = new(); + private readonly string _tempDir; + private readonly CapturingHubContext _hub = new(); + private readonly HandlerRoundState _rounds; + + private static readonly DateTimeOffset ResetsAt = new(2026, 9, 3, 2, 0, 0, TimeSpan.Zero); + + public HandlerRestartCoordinatorTests() + { + _tempDir = Path.Combine(Path.GetTempPath(), $"cd_handlerrestart_{Guid.NewGuid():N}"); + Directory.CreateDirectory(_tempDir); + _rounds = new HandlerRoundState(Path.Combine(_tempDir, "handler-rounds.json")); + } + + public void Dispose() { _db.Dispose(); try { Directory.Delete(_tempDir, true); } catch { } } + + private readonly List _waited = new(); + + private HandlerRestartCoordinator BuildCoordinator(UsageState usage) + { + var coordinator = new HandlerRestartCoordinator( + _db.CreateFactory(), usage, _rounds, new HubBroadcaster(_hub), + NullLogger.Instance); + coordinator.NowUtc = () => ResetsAt.AddHours(-3); + coordinator.Delay = (d, _) => { _waited.Add(d); return Task.CompletedTask; }; + return coordinator; + } + + /// The one signal a ConPTY death gives the worker: the 5h window pinned at the limit. + private static UsageState AtUsageLimit(double percent = 100) => + Snapshot(new UsageBucket(percent, ResetsAt)); + + private static UsageState Snapshot(UsageBucket? fiveHour) + { + var state = new UsageState(); + state.ReportSuccess(new UsageSnapshot(fiveHour, null, Array.Empty(), DateTime.UtcNow)); + return state; + } + + private int _nextNumber = 1; + + private async Task SeedHandlerTaskAsync(bool powerUseEnabled = true, bool powerUseAllowed = true) + { + var listId = Guid.NewGuid().ToString(); + var taskId = Guid.NewGuid().ToString(); + using (var ctx = _db.CreateContext()) + { + ctx.Lists.Add(new ListEntity { Id = listId, Name = "L", WorkingDir = _tempDir, CreatedAt = DateTime.UtcNow }); + ctx.ListConfigs.Add(new ListConfigEntity { ListId = listId, PowerUseAllowed = powerUseAllowed }); + ctx.Tasks.Add(new TaskEntity + { + Id = taskId, ListId = listId, Title = "Handler", Number = _nextNumber++, + IsManual = true, CreatedAt = DateTime.UtcNow, + }); + await ctx.SaveChangesAsync(); + + // Written straight to the row: AppSettingsRepository.UpdateAsync deliberately doesn't + // copy PowerUseEnabled (the usage monitor owns that toggle, not the settings modal). + await new AppSettingsRepository(ctx).GetAsync(); // materializes the singleton row + var settings = await ctx.AppSettings.FirstAsync(s => s.Id == AppSettingsEntity.SingletonId); + settings.PowerUseEnabled = powerUseEnabled; + await ctx.SaveChangesAsync(); + } + return taskId; + } + + private IReadOnlyList HandoffCalls => + _hub.Proxy.Calls.Where(c => c.Method == "HandoffRequested").ToList(); + + // ── acceptance 1: same phase, same task set, after the reset ──────────────────────────────── + + [Fact] + public async Task UsageLimitDeath_ReopensSamePhaseWithSameTaskSet_AfterTheReset() + { + var taskId = await SeedHandlerTaskAsync(); + _rounds.Save(new HandlerRound(taskId, MergeHelperPhase.Merge, new[] { "a", "b" }, DateTime.UtcNow)); + var coordinator = BuildCoordinator(AtUsageLimit()); + + Assert.True(await coordinator.OnSessionExitedAsync(taskId, 1)); + + var call = Assert.Single(HandoffCalls); + Assert.Equal(taskId, call.Args[0]); + Assert.Equal(new[] { "a", "b" }, (IReadOnlyList)call.Args[1]!); + Assert.Equal(MergeHelperPhase.Merge, call.Args[2]); + // Waited out the window instead of relaunching straight into the same limit. + Assert.Equal(new[] { TimeSpan.FromHours(3) }, _waited); + } + + // ── acceptance 2: at most one auto-restart per round ─────────────────────────────────────── + + [Fact] + public async Task SecondDeathInTheSameRound_DoesNotRestartAgain() + { + var taskId = await SeedHandlerTaskAsync(); + _rounds.Save(new HandlerRound(taskId, MergeHelperPhase.Merge, new[] { "a" }, DateTime.UtcNow)); + var coordinator = BuildCoordinator(AtUsageLimit()); + + Assert.True(await coordinator.OnSessionExitedAsync(taskId, 1)); + Assert.False(await coordinator.OnSessionExitedAsync(taskId, 1)); + + Assert.Single(HandoffCalls); + } + + // The restart relaunches the SAME phase, which writes the round again through + // InteractiveLaunchSpecService -- that re-save must not hand the round a fresh restart budget. + [Fact] + public async Task RestartRebuildingTheSamePhase_DoesNotRefreshTheRestartBudget() + { + var taskId = await SeedHandlerTaskAsync(); + _rounds.Save(new HandlerRound(taskId, MergeHelperPhase.Merge, new[] { "a" }, DateTime.UtcNow)); + var coordinator = BuildCoordinator(AtUsageLimit()); + + Assert.True(await coordinator.OnSessionExitedAsync(taskId, 1)); + // The relaunched session's launch spec writes the round again, same phase. + _rounds.Save(new HandlerRound(taskId, MergeHelperPhase.Merge, new[] { "a" }, DateTime.UtcNow)); + + Assert.False(await coordinator.OnSessionExitedAsync(taskId, 1)); + Assert.Single(HandoffCalls); + } + + // ── acceptance 3: PowerUse gates it ──────────────────────────────────────────────────────── + + [Fact] + public async Task ListWithoutPowerUseOptIn_NoRestart() + { + var taskId = await SeedHandlerTaskAsync(powerUseAllowed: false); + _rounds.Save(new HandlerRound(taskId, MergeHelperPhase.Merge, new[] { "a" }, DateTime.UtcNow)); + var coordinator = BuildCoordinator(AtUsageLimit()); + + Assert.False(await coordinator.OnSessionExitedAsync(taskId, 1)); + + Assert.Empty(HandoffCalls); + } + + [Fact] + public async Task PowerUseGloballyOff_NoRestart() + { + var taskId = await SeedHandlerTaskAsync(powerUseEnabled: false); + _rounds.Save(new HandlerRound(taskId, MergeHelperPhase.Merge, new[] { "a" }, DateTime.UtcNow)); + var coordinator = BuildCoordinator(AtUsageLimit()); + + Assert.False(await coordinator.OnSessionExitedAsync(taskId, 1)); + + Assert.Empty(HandoffCalls); + } + + // ── acceptance 4: a restart spends no handoff budget ─────────────────────────────────────── + + // handoff_list_handler is the only caller of HandoffRoundTracker.Register, and the restart never + // goes through it -- so a run interrupted by the usage limit does not lose a round of its + // wait/merge chain. Asserted against the tool the handler session itself reads the budget from. + [Fact] + public async Task Restart_DoesNotConsumeHandoffBudget() + { + var taskId = await SeedHandlerTaskAsync(); + _rounds.Save(new HandlerRound(taskId, MergeHelperPhase.Merge, new[] { "a" }, DateTime.UtcNow)); + + var tracker = new HandoffRoundTracker(); + tracker.Register(taskId, MergeHelperPhase.Merge, 4); // the run's real handoff into this phase + + using var ctx = _db.CreateContext(); + var tools = new HandoffMcpTools( + new TaskRepository(ctx), new HubBroadcaster(_hub), tracker, _db.CreateFactory()); + var before = await tools.GetHandoffStatus(taskId); + + Assert.True(await BuildCoordinator(AtUsageLimit()).OnSessionExitedAsync(taskId, 1)); + + var after = await tools.GetHandoffStatus(taskId); + Assert.Equal(before.Round, after.Round); + Assert.Equal(before.Remaining, after.Remaining); + Assert.False(after.Final); + } + + // ── proof requirement: a non-zero exit alone is not enough ───────────────────────────────── + + // The user closed the session -- exit 0 even sitting at the usage limit. + [Fact] + public async Task CleanExit_NoRestart() + { + var taskId = await SeedHandlerTaskAsync(); + _rounds.Save(new HandlerRound(taskId, MergeHelperPhase.Merge, new[] { "a" }, DateTime.UtcNow)); + + Assert.False(await BuildCoordinator(AtUsageLimit()).OnSessionExitedAsync(taskId, 0)); + Assert.Empty(HandoffCalls); + } + + [Fact] + public async Task NonZeroExit_WellBelowTheLimit_IsTreatedAsACrash_NoRestart() + { + var taskId = await SeedHandlerTaskAsync(); + _rounds.Save(new HandlerRound(taskId, MergeHelperPhase.Merge, new[] { "a" }, DateTime.UtcNow)); + + Assert.False(await BuildCoordinator(AtUsageLimit(percent: 40)).OnSessionExitedAsync(taskId, 1)); + Assert.Empty(HandoffCalls); + } + + [Fact] + public async Task NoUsageSnapshot_FailsClosed_NoRestart() + { + var taskId = await SeedHandlerTaskAsync(); + _rounds.Save(new HandlerRound(taskId, MergeHelperPhase.Merge, new[] { "a" }, DateTime.UtcNow)); + + Assert.False(await BuildCoordinator(new UsageState()).OnSessionExitedAsync(taskId, 1)); + Assert.Empty(HandoffCalls); + } + + // ── not a handler round at all ───────────────────────────────────────────────────────────── + + [Fact] + public async Task PlainInteractiveSession_HasNoRound_NoRestart() + { + var taskId = await SeedHandlerTaskAsync(); + + Assert.False(await BuildCoordinator(AtUsageLimit()).OnSessionExitedAsync(taskId, 1)); + Assert.Empty(HandoffCalls); + } + + // The triage round has no handoff launch spec, so there is nothing to reopen it with. + [Fact] + public async Task TriageRound_IsNotReopened() + { + var taskId = await SeedHandlerTaskAsync(); + _rounds.Save(new HandlerRound( + taskId, InteractiveLaunchSpecService.TriagePhase, new[] { "a" }, DateTime.UtcNow)); + + Assert.False(await BuildCoordinator(AtUsageLimit()).OnSessionExitedAsync(taskId, 1)); + Assert.Empty(HandoffCalls); + } +} diff --git a/tests/ClaudeDo.Worker.Tests/Runner/InteractiveLaunchSpecServiceTests.cs b/tests/ClaudeDo.Worker.Tests/Runner/InteractiveLaunchSpecServiceTests.cs index 80e9b453..9d8d41b5 100644 --- a/tests/ClaudeDo.Worker.Tests/Runner/InteractiveLaunchSpecServiceTests.cs +++ b/tests/ClaudeDo.Worker.Tests/Runner/InteractiveLaunchSpecServiceTests.cs @@ -73,12 +73,19 @@ public sealed class InteractiveLaunchSpecServiceTests : IDisposable => Task.FromResult>(Installed); } - private InteractiveLaunchSpecService BuildService(HandoffRoundTracker? handoffRounds = null) => + private InteractiveLaunchSpecService BuildService( + HandoffRoundTracker? handoffRounds = null, HandlerRoundState? roundState = null) => new(_db.CreateFactory(), _seeder, _registry, new WorktreeManager(new GitService(), _db.CreateFactory(), new WorkerConfig(), NullLogger.Instance), new GitService(), new WorkerConfig { ClaudeBin = _claudeStubPath }, - handoffRounds ?? new HandoffRoundTracker()); + handoffRounds ?? new HandoffRoundTracker(), + roundState ?? NewRoundState()); + + // Never the live ~/.claudeDo/handler-rounds.json -- a test run would otherwise write real + // handler rounds into the running app's state. + private HandlerRoundState NewRoundState() => + new(Path.Combine(_tempDir, $"handler-rounds-{Guid.NewGuid():N}.json")); private async Task SeedListAsync(string? workingDir = null, string name = "L") {