diff --git a/.planning/2026-07-17-architecture-deepening/task_plan.md b/.planning/2026-07-17-architecture-deepening/task_plan.md new file mode 100644 index 0000000..a352904 --- /dev/null +++ b/.planning/2026-07-17-architecture-deepening/task_plan.md @@ -0,0 +1,48 @@ +# Architecture deepening — three Strong candidates + +Branch: `refactor/deepen-modules`. Source: architecture review 2026-07-17 +(`%TEMP%\architecture-review-20260716-223837.html`). Vocabulary per +`.agents/skills/codebase-design` (module, interface, depth, seam, adapter, +leverage, locality); domain terms per `CONTEXT.md`. + +## Scope (in) + +Three Strong candidates, sequential commits, smallest first: + +1. **Active-branch query module.** Nine hand-rolled parentId walks across six + files (ChatSessionBootstrap, UsageFooter, SessionCompactionService, + AutoCompactionService, ChatSession, SessionTreeChoices, SessionForkService, + SlashCommandAdvanced import) get one home. Consumers stop walking the tree. +2. **One config mutation path.** Widen `ProviderConfigurator` until every + front-end's mutation fits (set-defaults-with-repair as used by + `providers use`/`models use`; the `/provider` slash repair policy routes + through the same seam). Delete `ConfigFileUpdater` (unvalidated splice). + LoginCommand consolidation is NOT in scope (that's card 6). +3. **One Turn event-consumption module.** Extract the AgentEvent consumption + loop shared by `ExecuteTurnCoreAsync` (text), `RunJsonTurnAsync` (JSON), + and `RpcHost.RunTurnAsync` into one module owning: artifact append, the + Failed → Completed("partial") terminal protocol, usage extraction, and + steering→follow-up promotion. Front-ends become presentation adapters. + Behaviour parity for REPL/JSON; RPC gains the previously-drifted + steering-promotion behaviour only if it fits the RPC contract cleanly — + otherwise parity and a noted follow-up. + +## Scope (out) + +- Card 4 (slash-command registry), card 5 (consume-vs-cut capabilities), + card 6 (OAuth login orchestration) — Worth-exploring cards, need a design + decision pass first. +- Appendix items except where a card touches them directly. +- No behaviour changes visible to users except drift removal in card 3's + consumers and (if clean) RPC steering promotion. + +## Acceptance criteria + +- `dotnet build WinHarness.sln -c Release` clean (TreatWarningsAsErrors gate). +- `dotnet test WinHarness.sln -c Release --no-build` green (5 non-Windows + skips expected — this box is Windows, so all should run). +- New modules covered by unit tests through their interfaces (no + InternalsVisibleTo additions, no reflection in tests). +- No public interface changes to IAgentRuntime's method signature; the + terminal-event protocol becomes documented on the extracted module. +- CONTEXT.md updated if new domain terms are named. diff --git a/src/WinHarness.Cli/Chat/ActiveBranch.cs b/src/WinHarness.Cli/Chat/ActiveBranch.cs new file mode 100644 index 0000000..85b8baa --- /dev/null +++ b/src/WinHarness.Cli/Chat/ActiveBranch.cs @@ -0,0 +1,110 @@ +using WinHarness.Conversation; +using WinHarness.Sessions; +using ConversationState = WinHarness.Conversation.Conversation; + +namespace WinHarness.Cli.Chat; + +/// +/// The active branch of a session, loaded once from root to leaf, answering the +/// branch queries the CLI needs: last-entry-of-kind, message counts, usage +/// totals, and flattening to conversation messages. +/// +internal sealed class ActiveBranch +{ + private readonly IReadOnlyList _entries; + + private ActiveBranch(IReadOnlyList entries) + { + _entries = entries; + } + + /// + /// Loads the active branch (root to leaf) from a session manager. + /// + public static ActiveBranch Load(ISessionManager sessionManager) + { + ArgumentNullException.ThrowIfNull(sessionManager); + return new ActiveBranch(sessionManager.GetActiveBranch()); + } + + /// + /// Wraps an already-ordered entry list, e.g. entries from a validated import file. + /// + public static ActiveBranch FromEntries(IReadOnlyList entries) + { + ArgumentNullException.ThrowIfNull(entries); + return new ActiveBranch(entries); + } + + /// + /// Gets the branch entries from root to leaf. + /// + public IReadOnlyList Entries => _entries; + + /// + /// Finds the most recent branch entry of type , or + /// when no entry matches. + /// + public T? LastOfType(Func? predicate = null) + where T : SessionEntry + { + for (int index = _entries.Count - 1; index >= 0; index--) + { + if (_entries[index] is T entry && (predicate is null || predicate(entry))) + { + return entry; + } + } + + return null; + } + + /// + /// Counts message entries on the branch. + /// + public int CountMessageEntries() => _entries.Count(static entry => entry is MessageSessionEntry); + + /// + /// Sums assistant-message usage over the branch; unset token counts add zero. + /// + public (long InputTokens, long OutputTokens) SumAssistantUsage() + { + long input = 0; + long output = 0; + foreach (SessionEntry entry in _entries) + { + if (entry is MessageSessionEntry { Message: { Role: ConversationRole.Assistant, Usage: { } usage } }) + { + input += usage.InputTokens ?? 0; + output += usage.OutputTokens ?? 0; + } + } + + return (input, output); + } + + /// + /// Flattens the branch into its conversation messages, in branch order. + /// + public List FlattenMessages() => + _entries + .OfType() + .Select(static entry => entry.Message) + .ToList(); + + /// + /// Sums message text length over a built conversation; the input for the + /// chars-per-token estimation heuristic. + /// + public static long SumMessageTextChars(ConversationState conversation) + { + ArgumentNullException.ThrowIfNull(conversation); + long total = 0; + foreach (ConversationMessage message in conversation.Messages) + { + total += message.Text.Length; + } + + return total; + } +} diff --git a/src/WinHarness.Cli/Chat/AutoCompactionService.cs b/src/WinHarness.Cli/Chat/AutoCompactionService.cs index 76910ad..f51149d 100644 --- a/src/WinHarness.Cli/Chat/AutoCompactionService.cs +++ b/src/WinHarness.Cli/Chat/AutoCompactionService.cs @@ -109,13 +109,8 @@ public static int ResolveContextWindow(WinHarnessOptions options, string provide /// public static long EstimateConversationTokens(ChatSession session) { - long chars = 0; - foreach (Conversation.ConversationMessage message in session.SessionManager - .BuildConversation(session.SelectedSkill?.SystemPrompt).Messages) - { - chars += message.Text.Length; - } - + long chars = ActiveBranch.SumMessageTextChars( + session.SessionManager.BuildConversation(session.SelectedSkill?.SystemPrompt)); return chars / CharsPerToken; } diff --git a/src/WinHarness.Cli/Chat/ChatSession.cs b/src/WinHarness.Cli/Chat/ChatSession.cs index 2048c74..a45aad5 100644 --- a/src/WinHarness.Cli/Chat/ChatSession.cs +++ b/src/WinHarness.Cli/Chat/ChatSession.cs @@ -79,7 +79,7 @@ public ChatSession( public bool IsEphemeral => !SessionManager.IsPersisted; public int CountActiveBranchMessages() => - SessionManager.GetActiveBranch().Count(static entry => entry is MessageSessionEntry); + ActiveBranch.Load(SessionManager).CountMessageEntries(); public void ReplaceSessionManager(ISessionManager sessionManager) { diff --git a/src/WinHarness.Cli/Chat/ChatSessionBootstrap.cs b/src/WinHarness.Cli/Chat/ChatSessionBootstrap.cs index 2577783..0ce51b0 100644 --- a/src/WinHarness.Cli/Chat/ChatSessionBootstrap.cs +++ b/src/WinHarness.Cli/Chat/ChatSessionBootstrap.cs @@ -214,15 +214,8 @@ private static async ValueTask OpenByPathOrIdAsync( internal static (string? ProviderId, string? ModelId) TryRestoreModelChange(ISessionManager sessionManager) { - for (int index = sessionManager.GetActiveBranch().Count - 1; index >= 0; index--) - { - if (sessionManager.GetActiveBranch()[index] is ModelChangeSessionEntry modelChange) - { - return (modelChange.ProviderId, modelChange.ModelId); - } - } - - return (null, null); + ModelChangeSessionEntry? modelChange = ActiveBranch.Load(sessionManager).LastOfType(); + return modelChange is null ? (null, null) : (modelChange.ProviderId, modelChange.ModelId); } } diff --git a/src/WinHarness.Cli/Chat/SessionCompactionService.cs b/src/WinHarness.Cli/Chat/SessionCompactionService.cs index 501ab1b..7e558e7 100644 --- a/src/WinHarness.Cli/Chat/SessionCompactionService.cs +++ b/src/WinHarness.Cli/Chat/SessionCompactionService.cs @@ -43,8 +43,8 @@ public async ValueTask CompactAsync( return CompactionResult.Failed("/compact requires a persisted session."); } - IReadOnlyList branch = session.GetActiveBranch(); - List messageEntries = branch.OfType().ToList(); + ActiveBranch branch = ActiveBranch.Load(session); + List messageEntries = branch.Entries.OfType().ToList(); if (messageEntries.Count < CompactionKeepMessageEntries) { return CompactionResult.Failed( @@ -52,7 +52,7 @@ public async ValueTask CompactAsync( } ConversationState before = session.BuildConversation(skillSystemPrompt); - int charsBefore = CountConversationChars(before); + long charsBefore = ActiveBranch.SumMessageTextChars(before); string summary = await RunSummarizationAsync( session, @@ -64,13 +64,16 @@ public async ValueTask CompactAsync( cancellationToken).ConfigureAwait(false); string firstKeptEntryId = messageEntries[^CompactionKeepMessageEntries].Id; - long? tokensBefore = FindLastUsageTokens(branch); + long? tokensBefore = branch + .LastOfType(static entry => + entry.Message.Role == ConversationRole.Assistant && entry.Message.Usage?.TotalTokens is not null) + ?.Message.Usage?.TotalTokens; await session.AppendCompactionAsync(summary, firstKeptEntryId, tokensBefore, cancellationToken) .ConfigureAwait(false); ConversationState after = session.BuildConversation(skillSystemPrompt); - int charsAfter = CountConversationChars(after); + long charsAfter = ActiveBranch.SumMessageTextChars(after); return CompactionResult.Completed(charsBefore, charsAfter); } @@ -125,42 +128,13 @@ private async ValueTask RunSummarizationAsync( return summary; } - - private static int CountConversationChars(ConversationState conversation) - { - int total = 0; - foreach (ConversationMessage message in conversation.Messages) - { - total += message.Text.Length; - } - - return total; - } - - private static long? FindLastUsageTokens(IReadOnlyList branch) - { - for (int index = branch.Count - 1; index >= 0; index--) - { - if (branch[index] is not MessageSessionEntry { Message.Role: ConversationRole.Assistant } messageEntry) - { - continue; - } - - if (messageEntry.Message.Usage?.TotalTokens is long tokens) - { - return tokens; - } - } - - return null; - } } -internal sealed record CompactionResult(bool Succeeded, string? Message, int CharsBefore = 0, int CharsAfter = 0) +internal sealed record CompactionResult(bool Succeeded, string? Message, long CharsBefore = 0, long CharsAfter = 0) { public static CompactionResult Failed(string message) => new(false, message); - public static CompactionResult Completed(int charsBefore, int charsAfter) => + public static CompactionResult Completed(long charsBefore, long charsAfter) => new( true, $"Compaction complete. Active context reduced from ~{charsBefore:N0} to ~{charsAfter:N0} characters.", diff --git a/src/WinHarness.Cli/Chat/SessionForkService.cs b/src/WinHarness.Cli/Chat/SessionForkService.cs index 25601c9..2f97533 100644 --- a/src/WinHarness.Cli/Chat/SessionForkService.cs +++ b/src/WinHarness.Cli/Chat/SessionForkService.cs @@ -31,10 +31,7 @@ public async ValueTask ForkAsync( ISessionManager forked = await _factory.CreateAsync(cwd, cancellationToken).ConfigureAwait(false); - List messages = source.GetActiveBranch() - .OfType() - .Select(static entry => entry.Message) - .ToList(); + List messages = ActiveBranch.Load(source).FlattenMessages(); if (messages.Count > 0) { diff --git a/src/WinHarness.Cli/Chat/SessionTreeChoices.cs b/src/WinHarness.Cli/Chat/SessionTreeChoices.cs index ff49e3f..fb5b8a6 100644 --- a/src/WinHarness.Cli/Chat/SessionTreeChoices.cs +++ b/src/WinHarness.Cli/Chat/SessionTreeChoices.cs @@ -74,7 +74,7 @@ public static IReadOnlyList ApplyBranch( onBranch(selected.Id); - int messageCount = sessionManager.GetActiveBranch().Count(static entry => entry is MessageSessionEntry); + int messageCount = ActiveBranch.Load(sessionManager).CountMessageEntries(); return [ $"Branched to entry {selected.Id}.", diff --git a/src/WinHarness.Cli/Chat/SlashCommandAdvanced.cs b/src/WinHarness.Cli/Chat/SlashCommandAdvanced.cs index 8d61010..47b91c9 100644 --- a/src/WinHarness.Cli/Chat/SlashCommandAdvanced.cs +++ b/src/WinHarness.Cli/Chat/SlashCommandAdvanced.cs @@ -108,10 +108,7 @@ public static async ValueTask ImportAsync( ISessionManager imported = await context.SessionFactory .CreateAsync(session.WorkspaceRoot, context.CancellationToken).ConfigureAwait(false); - List messages = entries - .OfType() - .Select(static entry => entry.Message) - .ToList(); + List messages = ActiveBranch.FromEntries(entries).FlattenMessages(); if (messages.Count > 0) { await imported.AppendMessagesAsync(messages, context.CancellationToken).ConfigureAwait(false); diff --git a/src/WinHarness.Cli/Chat/SlashCommandProcessor.cs b/src/WinHarness.Cli/Chat/SlashCommandProcessor.cs index 1e9483f..5354d07 100644 --- a/src/WinHarness.Cli/Chat/SlashCommandProcessor.cs +++ b/src/WinHarness.Cli/Chat/SlashCommandProcessor.cs @@ -2,6 +2,7 @@ using System.Linq; using Spectre.Console; using WinHarness.Configuration; +using WinHarness.Infrastructure.Configuration; using WinHarness.Sessions; namespace WinHarness.Cli.Chat; @@ -860,10 +861,7 @@ private static async ValueTask SwitchProviderAsync( } session.ProviderId = provider.Id; - if (!provider.Models.Any(model => string.Equals(model.Id, session.ModelId, StringComparison.OrdinalIgnoreCase))) - { - session.ModelId = provider.Models.Count > 0 ? provider.Models[0].Id : string.Empty; - } + session.ModelId = ProviderConfigurator.RepairDefaultModel(options, provider.Id, session.ModelId); await AppendModelChangeIfPersistedAsync(session, context).ConfigureAwait(false); return SlashCommandResult.Handled([$"Provider {session.ProviderId}, model {session.ModelId}."]); diff --git a/src/WinHarness.Cli/Chat/TurnPump.cs b/src/WinHarness.Cli/Chat/TurnPump.cs new file mode 100644 index 0000000..e927c48 --- /dev/null +++ b/src/WinHarness.Cli/Chat/TurnPump.cs @@ -0,0 +1,132 @@ +using WinHarness.Conversation; +using WinHarness.Runtime; + +namespace WinHarness.Cli.Chat; + +/// +/// Consumes one turn's stream: invokes a presentation +/// callback per event in stream order, appends the turn artifacts carried by +/// the terminal event to the session, and captures the failure message and +/// token usage. The front-ends (text REPL, JSON event stream, RPC host) plug +/// in as presentation adapters; this module owns everything else about reading +/// a turn. +/// +/// +/// Terminal-event protocol, emitted by SingleAgentRuntime and +/// interpreted only here: +/// +/// — the turn failed; the event +/// message is the failure reason. It may be followed by a partial +/// completion. +/// with +/// — follows a failure when assistant +/// text was streamed before the stop; carries the truncated turn artifacts +/// (the failed user message plus the partial assistant text) so partial work +/// is appended, not lost. +/// with +/// — success; carries the full turn +/// artifacts (user input, assistant segments, tool results). +/// +/// Cancellation is not interpreted here: +/// from the runtime propagates to the caller unchanged. +/// +internal sealed class TurnPump +{ + /// + /// carried by a normal (successful) + /// terminal event. + /// + internal const string NormalCompletionMessage = "completed"; + + /// + /// carried by the terminal + /// event that follows a + /// event when partial assistant text + /// was streamed; its artifacts are the truncated turn artifacts. + /// + internal const string PartialCompletionMessage = "partial"; + + private readonly ChatSession _session; + + public TurnPump(ChatSession session) + { + _session = session; + } + + /// + /// Runs one turn: streams events from , appends + /// the artifacts carried by a terminal + /// event (full artifacts on success, partial artifacts after a failure), + /// and invokes once per event in stream order, + /// at exactly the point the event is read. Returns what the turn produced; + /// whatever the runtime throws (including + /// ) propagates unchanged. + /// + public async ValueTask RunAsync( + AgentRunRequest request, + IAgentRuntime runtime, + Func present, + CancellationToken cancellationToken) + { + string? failureMessage = null; + MessageUsage? usage = null; + TurnArtifacts? appendedArtifacts = null; + bool appendedPartialArtifacts = false; + + await foreach (AgentEvent agentEvent in runtime.RunAsync(request, cancellationToken).ConfigureAwait(false)) + { + switch (agentEvent.Kind) + { + case AgentEventKind.Failed: + failureMessage = agentEvent.Message; + break; + + case AgentEventKind.Completed when agentEvent.TurnArtifacts is { } artifacts: + await _session.AppendTurnAsync(artifacts, cancellationToken).ConfigureAwait(false); + appendedArtifacts = artifacts; + appendedPartialArtifacts = string.Equals( + agentEvent.Message, + PartialCompletionMessage, + StringComparison.Ordinal); + usage = artifacts.Messages + .LastOrDefault(static message => message.Role == ConversationRole.Assistant) + ?.Usage; + break; + } + + await present(agentEvent).ConfigureAwait(false); + } + + return new TurnOutcome(failureMessage, usage, appendedArtifacts, appendedPartialArtifacts); + } + + /// + /// The post-turn steering policy: steering that never found a tool-round + /// injection point must not be lost — it is promoted to follow-up input so + /// it still runs. Front-ends with a follow-up concept (the REPL) call this + /// when a turn ends and when an aborted turn is torn down; front-ends + /// without one (RPC) leave unconsumed steering queued for the next turn. + /// + internal static void PromoteUnconsumedSteering(SteeringQueue steering, Queue followUps) + { + foreach (string queued in steering.DrainAll()) + { + followUps.Enqueue(queued); + } + } +} + +/// +/// What one pumped turn produced: the failure reason when the runtime reported +/// (null on success), the token usage from +/// the last assistant message of the appended artifacts, and the artifacts +/// appended to the session (null when the turn produced none). +/// distinguishes the truncated +/// artifacts of a completion +/// from the full artifacts of a normal one. +/// +internal sealed record TurnOutcome( + string? FailureMessage, + MessageUsage? Usage, + TurnArtifacts? AppendedArtifacts, + bool AppendedPartialArtifacts); diff --git a/src/WinHarness.Cli/Chat/UsageFooter.cs b/src/WinHarness.Cli/Chat/UsageFooter.cs index c3d3c82..e4db04b 100644 --- a/src/WinHarness.Cli/Chat/UsageFooter.cs +++ b/src/WinHarness.Cli/Chat/UsageFooter.cs @@ -44,38 +44,17 @@ public static string Format(ChatSession session, WinHarnessOptions options, Mess /// /// Sums assistant-message usage over the active branch. /// - public static (long InputTokens, long OutputTokens) SumSessionUsage(ChatSession session) - { - long input = 0; - long output = 0; - foreach (SessionEntry entry in session.SessionManager.GetActiveBranch()) - { - if (entry is MessageSessionEntry { Message: { Role: ConversationRole.Assistant, Usage: { } usage } }) - { - input += usage.InputTokens ?? 0; - output += usage.OutputTokens ?? 0; - } - } - - return (input, output); - } + public static (long InputTokens, long OutputTokens) SumSessionUsage(ChatSession session) => + ActiveBranch.Load(session.SessionManager).SumAssistantUsage(); /// /// Finds the most recent assistant usage on the active branch. /// - public static MessageUsage? FindLastTurnUsage(ChatSession session) - { - IReadOnlyList branch = session.SessionManager.GetActiveBranch(); - for (int index = branch.Count - 1; index >= 0; index--) - { - if (branch[index] is MessageSessionEntry { Message: { Role: ConversationRole.Assistant, Usage: { } usage } }) - { - return usage; - } - } - - return null; - } + public static MessageUsage? FindLastTurnUsage(ChatSession session) => + ActiveBranch.Load(session.SessionManager) + .LastOfType(static entry => + entry.Message is { Role: ConversationRole.Assistant, Usage: not null }) + ?.Message.Usage; /// /// Renders token counts compactly: 950 → "950", 30_400 → "30.4k", 1_000_000 → "1.0m". diff --git a/src/WinHarness.Cli/Configuration/ConfigFileUpdater.cs b/src/WinHarness.Cli/Configuration/ConfigFileUpdater.cs deleted file mode 100644 index cf93bcc..0000000 --- a/src/WinHarness.Cli/Configuration/ConfigFileUpdater.cs +++ /dev/null @@ -1,82 +0,0 @@ -using System.Buffers; -using System.Text.Json; -using WinHarness.Configuration; -using WinHarness.Infrastructure.Configuration; - -namespace WinHarness.Cli.Configuration; - -/// -/// Atomically updates root-level string properties in config.json. -/// -internal static class ConfigFileUpdater -{ - public static async ValueTask SetRootStringPropertyAsync( - string propertyName, - string value, - CancellationToken cancellationToken) - { - await SetRootStringPropertiesAsync( - new Dictionary { [propertyName] = value }, - cancellationToken).ConfigureAwait(false); - } - - /// - /// Atomically updates multiple root-level string properties in config.json, - /// preserving all other properties. - /// - public static async ValueTask SetRootStringPropertiesAsync( - IReadOnlyDictionary updates, - CancellationToken cancellationToken) - { - string directory = WinHarnessConfiguration.GetConfigurationDirectory(); - Directory.CreateDirectory(directory); - string path = Path.Combine(directory, "config.json"); - - JsonDocument? document = File.Exists(path) - ? JsonDocument.Parse(await File.ReadAllTextAsync(path, cancellationToken).ConfigureAwait(false)) - : null; - - ArrayBufferWriter buffer = new(); - try - { - using (Utf8JsonWriter writer = new(buffer, new JsonWriterOptions { Indented = true })) - { - writer.WriteStartObject(); - var written = new HashSet(StringComparer.Ordinal); - - if (document is not null && document.RootElement.ValueKind == JsonValueKind.Object) - { - foreach (JsonProperty property in document.RootElement.EnumerateObject()) - { - if (updates.TryGetValue(property.Name, out string? replacement)) - { - writer.WriteString(property.Name, replacement); - written.Add(property.Name); - } - else - { - writer.WritePropertyName(property.Name); - property.Value.WriteTo(writer); - } - } - } - - foreach ((string key, string value) in updates) - { - if (!written.Contains(key)) - { - writer.WriteString(key, value); - } - } - - writer.WriteEndObject(); - } - } - finally - { - document?.Dispose(); - } - - await AtomicFile.WriteAllBytesAsync(path, buffer.WrittenMemory.ToArray(), cancellationToken).ConfigureAwait(false); - } -} diff --git a/src/WinHarness.Cli/Program.cs b/src/WinHarness.Cli/Program.cs index e49d099..b38f402 100644 --- a/src/WinHarness.Cli/Program.cs +++ b/src/WinHarness.Cli/Program.cs @@ -495,39 +495,12 @@ await sink.WriteAsync( app.Add("providers use", async (string providerId, CancellationToken cancellationToken) => { WinHarnessOptions options = host.Services.GetRequiredService(); - ProviderOptions? targetProvider = options.Providers.FirstOrDefault(provider => - string.Equals(provider.Id, providerId, StringComparison.OrdinalIgnoreCase)); - if (targetProvider is null) - { - throw new InvalidOperationException($"Provider '{providerId}' is not configured."); - } + ProviderConfigurator configurator = host.Services.GetRequiredService(); - // Ensure the current default model is valid under the new provider. - // If it isn't, pick the first available model from the target provider - // or clear the default model so the CLI doesn't fail validation on next start. - string? resolvedModel = options.DefaultModel; - if (options.DefaultModel.Length > 0) - { - bool modelExists = targetProvider.Models.Any(model => - string.Equals(model.Id, options.DefaultModel, StringComparison.OrdinalIgnoreCase)); - if (!modelExists) - { - resolvedModel = targetProvider.Models.Count > 0 - ? targetProvider.Models[0].Id - : string.Empty; - } - } + string resolvedModel = await configurator.SetDefaultProviderAsync(providerId, cancellationToken).ConfigureAwait(false); if (resolvedModel != options.DefaultModel) { - await ConfigFileUpdater.SetRootStringPropertiesAsync( - new Dictionary - { - ["defaultProvider"] = providerId, - ["defaultModel"] = resolvedModel - }, - cancellationToken).ConfigureAwait(false); - if (resolvedModel.Length > 0) { Console.WriteLine($"Default provider set to {providerId}, model set to {resolvedModel}."); @@ -539,7 +512,6 @@ await ConfigFileUpdater.SetRootStringPropertiesAsync( } else { - await ConfigFileUpdater.SetRootStringPropertyAsync("defaultProvider", providerId, cancellationToken).ConfigureAwait(false); Console.WriteLine($"Default provider set to {providerId}."); } }); @@ -599,47 +571,17 @@ await ConfigFileUpdater.SetRootStringPropertiesAsync( app.Add("models use", async (string modelId, string? providerId = null, CancellationToken cancellationToken = default) => { - WinHarnessOptions options = host.Services.GetRequiredService(); + ProviderConfigurator configurator = host.Services.GetRequiredService(); // When --provider-id is given, switch both provider and model atomically. if (providerId is not null) { - ProviderOptions? targetProvider = options.Providers.FirstOrDefault(candidate => - string.Equals(candidate.Id, providerId, StringComparison.OrdinalIgnoreCase)); - if (targetProvider is null) - { - throw new InvalidOperationException($"Provider '{providerId}' is not configured."); - } - - if (!targetProvider.Models.Any(model => string.Equals(model.Id, modelId, StringComparison.OrdinalIgnoreCase))) - { - throw new InvalidOperationException($"Model '{modelId}' is not configured for provider '{providerId}'."); - } - - await ConfigFileUpdater.SetRootStringPropertiesAsync( - new Dictionary - { - ["defaultProvider"] = providerId, - ["defaultModel"] = modelId - }, - cancellationToken).ConfigureAwait(false); + await configurator.SetDefaultsAsync(providerId, modelId, cancellationToken).ConfigureAwait(false); Console.WriteLine($"Default provider set to {providerId}, model set to {modelId}."); return; } - ProviderOptions? provider = options.Providers.FirstOrDefault(candidate => - string.Equals(candidate.Id, options.DefaultProvider, StringComparison.OrdinalIgnoreCase)); - if (provider is null) - { - throw new InvalidOperationException("Configure a default provider before selecting a model."); - } - - if (!provider.Models.Any(model => string.Equals(model.Id, modelId, StringComparison.OrdinalIgnoreCase))) - { - throw new InvalidOperationException($"Model '{modelId}' is not configured for provider '{provider.Id}'."); - } - - await ConfigFileUpdater.SetRootStringPropertyAsync("defaultModel", modelId, cancellationToken).ConfigureAwait(false); + await configurator.SetDefaultModelAsync(modelId, cancellationToken).ConfigureAwait(false); Console.WriteLine($"Default model set to {modelId}."); }); @@ -972,24 +914,24 @@ public static async ValueTask RunJsonTurnAsync( IAgentRuntime runtime = services.GetRequiredService(); Conversation runConversation = session.CreateRunConversation(prompt); + AgentRunRequest request = new( + session.ProviderId, + session.ModelId, + runConversation, + session.WorkspaceRoot, + session.ProjectContext, + session.ReasoningEffort, + session.ToolFilter); static void Emit(JsonChatEvent chatEvent) => Console.Out.WriteLine(JsonSerializer.Serialize(chatEvent, JsonChatEventContext.Default.JsonChatEvent)); Emit(JsonChatEvent.TurnStart(session.ProviderId, session.ModelId)); - bool failed = false; StringBuilder assistantText = new(); - await foreach (AgentEvent agentEvent in runtime.RunAsync( - new AgentRunRequest( - session.ProviderId, - session.ModelId, - runConversation, - session.WorkspaceRoot, - session.ProjectContext, - session.ReasoningEffort, - session.ToolFilter), - cancellationToken).ConfigureAwait(false)) + // Presentation adapter: serializes each event at the point the pump + // reads it. Artifact appends and the failure flag are the pump's. + ValueTask Present(AgentEvent agentEvent) { switch (agentEvent.Kind) { @@ -1003,31 +945,29 @@ static void Emit(JsonChatEvent chatEvent) => break; case AgentEventKind.Failed: - failed = true; Emit(JsonChatEvent.FromError(agentEvent.Message)); break; - case AgentEventKind.Completed: - if (agentEvent.TurnArtifacts is { } artifacts) + case AgentEventKind.Completed when agentEvent.TurnArtifacts is { } artifacts: + ConversationMessage? assistant = artifacts.Messages + .LastOrDefault(static message => message.Role == ConversationRole.Assistant); + Emit(JsonChatEvent.AssistantMessage(assistant?.Text ?? assistantText.ToString())); + if (assistant?.Usage is { } usage) { - await session.AppendTurnAsync(artifacts, cancellationToken).ConfigureAwait(false); - ConversationMessage? assistant = artifacts.Messages - .LastOrDefault(static message => message.Role == ConversationRole.Assistant); - Emit(JsonChatEvent.AssistantMessage(assistant?.Text ?? assistantText.ToString())); - if (assistant?.Usage is { } usage) - { - Emit(JsonChatEvent.Usage(usage.InputTokens, usage.OutputTokens)); - } + Emit(JsonChatEvent.Usage(usage.InputTokens, usage.OutputTokens)); } break; - - default: - break; } + + return ValueTask.CompletedTask; } - if (!failed) + TurnOutcome outcome = await new TurnPump(session) + .RunAsync(request, runtime, Present, cancellationToken) + .ConfigureAwait(false); + + if (outcome.FailureMessage is null) { Emit(JsonChatEvent.TurnEnd()); } @@ -1574,10 +1514,7 @@ private static async ValueTask RunTurnWithSteeringAsync( await turnCts.CancelAsync().ConfigureAwait(false); // Restore unsent steering messages as follow-up input. - foreach (string queued in session.Steering.DrainAll()) - { - followUps.Enqueue(queued); - } + TurnPump.PromoteUnconsumedSteering(session.Steering, followUps); pending.Clear(); break; @@ -1598,10 +1535,7 @@ private static async ValueTask RunTurnWithSteeringAsync( await turnCts.CancelAsync().ConfigureAwait(false); // Restore unsent steering messages as follow-up input. - foreach (string queued in session.Steering.DrainAll()) - { - followUps.Enqueue(queued); - } + TurnPump.PromoteUnconsumedSteering(session.Steering, followUps); pending.Clear(); break; @@ -1643,10 +1577,7 @@ private static async ValueTask RunTurnWithSteeringAsync( // lost — promote it to follow-up so it still runs. if (!aborted) { - foreach (string queued in session.Steering.DrainAll()) - { - followUps.Enqueue(queued); - } + TurnPump.PromoteUnconsumedSteering(session.Steering, followUps); } await AwaitTurnAsync(turn, cancellationToken).ConfigureAwait(false); @@ -2288,6 +2219,15 @@ private static async ValueTask RunTurnAsync( { IAgentRuntime runtime = services.GetRequiredService(); Conversation runConversation = session.CreateRunConversation(prompt); + AgentRunRequest request = new( + session.ProviderId, + session.ModelId, + runConversation, + session.WorkspaceRoot, + session.ProjectContext, + session.ReasoningEffort, + session.ToolFilter, + session.Steering); bool interactive = !Console.IsOutputRedirected; @@ -2310,12 +2250,10 @@ private static async ValueTask RunTurnAsync( bool rawLabelWritten = false; bool plainLabelWritten = false; - // Tracks whether the turn produced any assistant text or ended in a failure, - // so an empty provider completion (stream closed with no content) can be - // surfaced to the user instead of silently returning to the prompt. + // Tracks whether the turn produced any assistant text, so an empty + // provider completion (stream closed with no content and no failure) + // can be surfaced to the user instead of silently returning to the prompt. bool producedAssistantText = false; - bool turnFailed = false; - string? failureMessage = null; // Ends the current assistant text segment. In markdown mode the spinner is // stopped and the buffered segment is rendered as formatted markdown; in raw @@ -2348,135 +2286,120 @@ async ValueTask FinalizeSegmentAsync() writer = new AssistantStreamWriter(); } - if (interactive) + // Presentation adapter: renders each event at the point the pump reads + // it. Artifact appends, the failure message, and usage are the pump's. + async ValueTask Present(AgentEvent agentEvent) { - thinking.Start(); - } - - try - { - await foreach (AgentEvent agentEvent in runtime.RunAsync( - new AgentRunRequest( - session.ProviderId, - session.ModelId, - runConversation, - session.WorkspaceRoot, - session.ProjectContext, - session.ReasoningEffort, - session.ToolFilter, - session.Steering), - cancellationToken).ConfigureAwait(false)) + switch (agentEvent.Kind) { - switch (agentEvent.Kind) - { - case AgentEventKind.ToolActivity: - if (interactive) - { - await FinalizeSegmentAsync().ConfigureAwait(false); + case AgentEventKind.ToolActivity: + if (interactive) + { + await FinalizeSegmentAsync().ConfigureAwait(false); - if (agentEvent.ToolActivity is { } info) - { - if (verbose) - { - await thinking.StopAsync().ConfigureAwait(false); - } - - toolBatch.OnEvent(info); - if (!verbose) - { - thinking.SetLabel(toolBatch.LiveLabel); - } - } - else + if (agentEvent.ToolActivity is { } info) + { + if (verbose) { await thinking.StopAsync().ConfigureAwait(false); - AnsiConsole.MarkupLine("[dim]" + Markup.Escape(agentEvent.Message) + "[/]"); - thinking.SetLabel("thinking"); } - thinking.Start(); + toolBatch.OnEvent(info); + if (!verbose) + { + thinking.SetLabel(toolBatch.LiveLabel); + } } - - break; - - case AgentEventKind.Failed: - turnFailed = true; - failureMessage = agentEvent.Message; - if (interactive) + else { - await FinalizeSegmentAsync().ConfigureAwait(false); await thinking.StopAsync().ConfigureAwait(false); - toolBatch.Settle(); + AnsiConsole.MarkupLine("[dim]" + Markup.Escape(agentEvent.Message) + "[/]"); + thinking.SetLabel("thinking"); } - AnsiConsole.MarkupLine("[red]" + Markup.Escape(agentEvent.Message) + "[/]"); - break; + thinking.Start(); + } - case AgentEventKind.Completed: - if (agentEvent.TurnArtifacts is not null) - { - await session.AppendTurnAsync(agentEvent.TurnArtifacts, cancellationToken) - .ConfigureAwait(false); - } + break; - break; + case AgentEventKind.Failed: + if (interactive) + { + await FinalizeSegmentAsync().ConfigureAwait(false); + await thinking.StopAsync().ConfigureAwait(false); + toolBatch.Settle(); + } - case AgentEventKind.AssistantDelta: - if (interactive && toolBatch.HasPendingBatch) - { - await thinking.StopAsync().ConfigureAwait(false); - toolBatch.Settle(); - thinking.SetLabel("thinking"); - thinking.Start(); - } + AnsiConsole.MarkupLine("[red]" + Markup.Escape(agentEvent.Message) + "[/]"); + break; - assistantBuffer.Append(agentEvent.Message); - if (!string.IsNullOrEmpty(agentEvent.Message)) - { - producedAssistantText = true; - } + case AgentEventKind.AssistantDelta: + if (interactive && toolBatch.HasPendingBatch) + { + await thinking.StopAsync().ConfigureAwait(false); + toolBatch.Settle(); + thinking.SetLabel("thinking"); + thinking.Start(); + } - if (interactive) - { - segmentActive = true; - segmentBuffer.Append(agentEvent.Message); + assistantBuffer.Append(agentEvent.Message); + if (!string.IsNullOrEmpty(agentEvent.Message)) + { + producedAssistantText = true; + } - if (!session.RenderMarkdown) - { - // Raw streaming: drop the spinner on first token, then - // emit tokens as they arrive. - if (!rawLabelWritten) - { - await thinking.StopAsync().ConfigureAwait(false); - rawLabelWritten = true; - } - - writer.Write(agentEvent.Message); - } - } - else if (!session.RenderMarkdown) + if (interactive) + { + segmentActive = true; + segmentBuffer.Append(agentEvent.Message); + + if (!session.RenderMarkdown) { - if (!plainLabelWritten) + // Raw streaming: drop the spinner on first token, then + // emit tokens as they arrive. + if (!rawLabelWritten) { - AnsiConsole.Markup("[bold blue]•[/] "); - plainLabelWritten = true; + await thinking.StopAsync().ConfigureAwait(false); + rawLabelWritten = true; } - Console.Write(agentEvent.Message); + writer.Write(agentEvent.Message); + } + } + else if (!session.RenderMarkdown) + { + if (!plainLabelWritten) + { + AnsiConsole.Markup("[bold blue]•[/] "); + plainLabelWritten = true; } - break; + Console.Write(agentEvent.Message); + } - default: - break; - } + break; } } + + if (interactive) + { + thinking.Start(); + } + + TurnOutcome outcome; + try + { + outcome = await new TurnPump(session) + .RunAsync(request, runtime, Present, cancellationToken) + .ConfigureAwait(false); + } finally { await thinking.StopAsync().ConfigureAwait(false); } + bool turnFailed = outcome.FailureMessage is not null; + if (interactive) { await FinalizeSegmentAsync().ConfigureAwait(false); @@ -2490,7 +2413,7 @@ await session.AppendTurnAsync(agentEvent.TurnArtifacts, cancellationToken) AnsiConsole.MarkupLine("[yellow]The model returned an empty response. Try resending, or switch models with /model.[/]"); } - return failureMessage; + return outcome.FailureMessage; } if (session.RenderMarkdown) @@ -2512,6 +2435,6 @@ await session.AppendTurnAsync(agentEvent.TurnArtifacts, cancellationToken) Console.Error.WriteLine("The model returned an empty response."); } - return failureMessage; + return outcome.FailureMessage; } } diff --git a/src/WinHarness.Cli/Rpc/RpcHost.cs b/src/WinHarness.Cli/Rpc/RpcHost.cs index 9d6f3bb..728f210 100644 --- a/src/WinHarness.Cli/Rpc/RpcHost.cs +++ b/src/WinHarness.Cli/Rpc/RpcHost.cs @@ -157,61 +157,66 @@ private async Task RunTurnAsync( ConversationState runConversation, CancellationToken turnToken) { - bool failed = false; - try + AgentRunRequest request = new( + session.ProviderId, + session.ModelId, + runConversation, + session.WorkspaceRoot, + session.ProjectContext, + session.ReasoningEffort, + session.ToolFilter, + session.Steering); + + // Presentation adapter: wraps each event as an RpcEvent at the point + // the pump reads it. Artifact appends and the failure flag are the + // pump's. + ValueTask Present(AgentEvent agentEvent) { - await foreach (AgentEvent agentEvent in runtime.RunAsync( - new AgentRunRequest( - session.ProviderId, - session.ModelId, - runConversation, - session.WorkspaceRoot, - session.ProjectContext, - session.ReasoningEffort, - session.ToolFilter, - session.Steering), - turnToken).ConfigureAwait(false)) + switch (agentEvent.Kind) { - switch (agentEvent.Kind) - { - case AgentEventKind.AssistantDelta: - EmitEvent(requestId, JsonChatEvent.AssistantDelta(agentEvent.Message)); - break; - case AgentEventKind.ToolActivity when agentEvent.ToolActivity is { } info: - EmitEvent(requestId, JsonChatEvent.Tool(info)); - break; - case AgentEventKind.Failed: - failed = true; - EmitEvent(requestId, JsonChatEvent.FromError(agentEvent.Message)); - break; - case AgentEventKind.Completed when agentEvent.TurnArtifacts is { } artifacts: - await session.AppendTurnAsync(artifacts, turnToken).ConfigureAwait(false); - ConversationMessage? assistant = artifacts.Messages - .LastOrDefault(static message => message.Role == ConversationRole.Assistant); - EmitEvent(requestId, JsonChatEvent.AssistantMessage(assistant?.Text ?? string.Empty)); - if (assistant?.Usage is { } usage) - { - EmitEvent(requestId, JsonChatEvent.Usage(usage.InputTokens, usage.OutputTokens)); - } - - break; - default: - break; - } + case AgentEventKind.AssistantDelta: + EmitEvent(requestId, JsonChatEvent.AssistantDelta(agentEvent.Message)); + break; + case AgentEventKind.ToolActivity when agentEvent.ToolActivity is { } info: + EmitEvent(requestId, JsonChatEvent.Tool(info)); + break; + case AgentEventKind.Failed: + EmitEvent(requestId, JsonChatEvent.FromError(agentEvent.Message)); + break; + case AgentEventKind.Completed when agentEvent.TurnArtifacts is { } artifacts: + ConversationMessage? assistant = artifacts.Messages + .LastOrDefault(static message => message.Role == ConversationRole.Assistant); + EmitEvent(requestId, JsonChatEvent.AssistantMessage(assistant?.Text ?? string.Empty)); + if (assistant?.Usage is { } usage) + { + EmitEvent(requestId, JsonChatEvent.Usage(usage.InputTokens, usage.OutputTokens)); + } + + break; } + + return ValueTask.CompletedTask; + } + + TurnOutcome outcome; + try + { + outcome = await new TurnPump(session) + .RunAsync(request, runtime, Present, turnToken) + .ConfigureAwait(false); } catch (OperationCanceledException) { - failed = true; EmitEvent(requestId, JsonChatEvent.FromError("Turn aborted.")); + return; } catch (Exception ex) { - failed = true; EmitEvent(requestId, JsonChatEvent.FromError(ex.Message)); + return; } - if (!failed) + if (outcome.FailureMessage is null) { EmitEvent(requestId, JsonChatEvent.TurnEnd()); } diff --git a/src/WinHarness.Infrastructure/Configuration/ProviderConfigurator.cs b/src/WinHarness.Infrastructure/Configuration/ProviderConfigurator.cs index 315f91f..d5da6bc 100644 --- a/src/WinHarness.Infrastructure/Configuration/ProviderConfigurator.cs +++ b/src/WinHarness.Infrastructure/Configuration/ProviderConfigurator.cs @@ -240,6 +240,44 @@ public async ValueTask RemoveProviderAsync(string providerId, CancellationToken } } + /// + /// Sets the default provider. The default model is repaired when it does not + /// belong to the new provider (see ). Returns + /// the resulting default model id so callers can report whether it changed. + /// + public async ValueTask SetDefaultProviderAsync(string providerId, CancellationToken cancellationToken) + { + ArgumentException.ThrowIfNullOrWhiteSpace(providerId); + + WinHarnessOptions options = await _store.LoadAsync(cancellationToken).ConfigureAwait(false); + ProviderOptions provider = FindProviderOrThrow(options, providerId); + + options.DefaultProvider = provider.Id; + options.DefaultModel = RepairDefaultModel(options, provider.Id, options.DefaultModel); + + await _store.SaveAsync(options, cancellationToken).ConfigureAwait(false); + return options.DefaultModel; + } + + /// + /// Sets the default model. The model must belong to the current default + /// provider. + /// + public async ValueTask SetDefaultModelAsync(string modelId, CancellationToken cancellationToken) + { + ArgumentException.ThrowIfNullOrWhiteSpace(modelId); + + WinHarnessOptions options = await _store.LoadAsync(cancellationToken).ConfigureAwait(false); + ProviderOptions provider = options.Providers.FirstOrDefault(candidate => + string.Equals(candidate.Id, options.DefaultProvider, StringComparison.OrdinalIgnoreCase)) + ?? throw new InvalidOperationException("Configure a default provider before selecting a model."); + + ModelOptions model = FindModelOrThrow(provider, modelId); + options.DefaultModel = model.Id; + + await _store.SaveAsync(options, cancellationToken).ConfigureAwait(false); + } + /// /// Sets the default provider and (optionally) the default model. /// @@ -249,23 +287,45 @@ public async ValueTask SetDefaultsAsync( CancellationToken cancellationToken) { WinHarnessOptions options = await _store.LoadAsync(cancellationToken).ConfigureAwait(false); - ProviderOptions provider = options.Providers.FirstOrDefault(candidate => - string.Equals(candidate.Id, providerId, StringComparison.OrdinalIgnoreCase)) - ?? throw new InvalidOperationException($"Provider '{providerId}' is not configured."); + ProviderOptions provider = FindProviderOrThrow(options, providerId); options.DefaultProvider = provider.Id; if (modelId is not null) { - ModelOptions model = provider.Models.FirstOrDefault(candidate => - string.Equals(candidate.Id, modelId, StringComparison.OrdinalIgnoreCase)) - ?? throw new InvalidOperationException($"Model '{modelId}' is not configured for provider '{provider.Id}'."); + ModelOptions model = FindModelOrThrow(provider, modelId); options.DefaultModel = model.Id; } await _store.SaveAsync(options, cancellationToken).ConfigureAwait(false); } + /// + /// Resolves the default model after a provider switch: keeps + /// when it belongs to + /// , otherwise falls back to the provider's + /// first model (or empty when it has none). An empty current model is left + /// alone so an unset default stays unset. + /// + public static string RepairDefaultModel(WinHarnessOptions options, string providerId, string currentModelId) + { + ProviderOptions provider = FindProviderOrThrow(options, providerId); + + if (currentModelId.Length == 0) + { + return currentModelId; + } + + bool modelExists = provider.Models.Any(model => + string.Equals(model.Id, currentModelId, StringComparison.OrdinalIgnoreCase)); + if (modelExists) + { + return currentModelId; + } + + return provider.Models.Count > 0 ? provider.Models[0].Id : string.Empty; + } + /// /// Derives a credential target name for a provider id. /// @@ -273,4 +333,18 @@ public static string BuildCredentialName(string providerId) { return "WinHarness:" + providerId; } + + private static ProviderOptions FindProviderOrThrow(WinHarnessOptions options, string providerId) + { + return options.Providers.FirstOrDefault(candidate => + string.Equals(candidate.Id, providerId, StringComparison.OrdinalIgnoreCase)) + ?? throw new InvalidOperationException($"Provider '{providerId}' is not configured."); + } + + private static ModelOptions FindModelOrThrow(ProviderOptions provider, string modelId) + { + return provider.Models.FirstOrDefault(candidate => + string.Equals(candidate.Id, modelId, StringComparison.OrdinalIgnoreCase)) + ?? throw new InvalidOperationException($"Model '{modelId}' is not configured for provider '{provider.Id}'."); + } } diff --git a/tests/WinHarness.IntegrationTests/ActiveBranchTests.cs b/tests/WinHarness.IntegrationTests/ActiveBranchTests.cs new file mode 100644 index 0000000..0ffcf04 --- /dev/null +++ b/tests/WinHarness.IntegrationTests/ActiveBranchTests.cs @@ -0,0 +1,230 @@ +using Microsoft.VisualStudio.TestTools.UnitTesting; +using WinHarness.Cli.Chat; +using WinHarness.Conversation; +using WinHarness.Infrastructure.Sessions; +using WinHarness.Sessions; +using ConversationState = WinHarness.Conversation.Conversation; + +namespace WinHarness.IntegrationTests; + +[TestClass] +public sealed class ActiveBranchTests +{ + private string _sessionsRoot = null!; + + [TestInitialize] + public void SetUp() + { + _sessionsRoot = Path.Combine(Path.GetTempPath(), "WinHarnessActiveBranch", Guid.NewGuid().ToString("N")); + } + + [TestCleanup] + public void TearDown() + { + if (Directory.Exists(_sessionsRoot)) + { + Directory.Delete(_sessionsRoot, recursive: true); + } + } + + [TestMethod] + public async Task LastOfTypeReturnsNullOnEmptyBranch() + { + ISessionManager session = await CreateSessionAsync(); + + Assert.IsNull(ActiveBranch.Load(session).LastOfType()); + } + + [TestMethod] + public async Task LastOfTypeReturnsNullWhenKindAbsent() + { + ISessionManager session = await CreateSessionAsync(); + await session.AppendMessagesAsync( + [ + ConversationMessage.FromText(ConversationRole.User, "one"), + ConversationMessage.FromText(ConversationRole.Assistant, "two"), + ], + CancellationToken.None); + + Assert.IsNull(ActiveBranch.Load(session).LastOfType()); + } + + [TestMethod] + public async Task LastOfTypeReturnsMostRecentMatch() + { + ISessionManager session = await CreateSessionAsync(); + await session.AppendModelChangeAsync("local", "first-model", CancellationToken.None); + await session.AppendMessagesAsync( + [ConversationMessage.FromText(ConversationRole.User, "between")], + CancellationToken.None); + await session.AppendModelChangeAsync("local", "second-model", CancellationToken.None); + + ModelChangeSessionEntry? found = ActiveBranch.Load(session).LastOfType(); + + Assert.IsNotNull(found); + Assert.AreEqual("second-model", found.ModelId); + } + + [TestMethod] + public async Task LastOfTypeWithPredicateSkipsNonMatchingEntries() + { + ISessionManager session = await CreateSessionAsync(); + await session.AppendMessagesAsync( + [ + ConversationMessage.FromText(ConversationRole.Assistant, "with usage") with + { + Usage = new MessageUsage(10, 5, 15) + }, + ConversationMessage.FromText(ConversationRole.Assistant, "without usage"), + ], + CancellationToken.None); + + MessageSessionEntry? found = ActiveBranch.Load(session) + .LastOfType(static entry => entry.Message.Usage is not null); + + Assert.IsNotNull(found); + Assert.AreEqual("with usage", found.Message.Text); + } + + [TestMethod] + public async Task CountMessageEntriesCountsOnlyMessages() + { + ISessionManager session = await CreateSessionAsync(); + await session.AppendMessagesAsync( + [ + ConversationMessage.FromText(ConversationRole.User, "one"), + ConversationMessage.FromText(ConversationRole.Assistant, "two"), + ], + CancellationToken.None); + await session.AppendModelChangeAsync("local", "coder", CancellationToken.None); + await session.AppendSessionInfoAsync("named", CancellationToken.None); + + Assert.AreEqual(2, ActiveBranch.Load(session).CountMessageEntries()); + } + + [TestMethod] + public async Task QueriesReflectBranchedState() + { + ISessionManager session = await CreateSessionAsync(); + string rootId = await session.AppendMessagesAsync( + [ConversationMessage.FromText(ConversationRole.User, "root")], + CancellationToken.None); + await session.AppendMessagesAsync( + [ConversationMessage.FromText(ConversationRole.Assistant, "child")], + CancellationToken.None); + await session.AppendMessagesAsync( + [ConversationMessage.FromText(ConversationRole.User, "grandchild")], + CancellationToken.None); + + session.BranchTo(rootId); + ActiveBranch branch = ActiveBranch.Load(session); + + Assert.AreEqual(1, branch.CountMessageEntries()); + Assert.AreEqual("root", branch.FlattenMessages()[0].Text); + } + + [TestMethod] + public async Task SumAssistantUsageSumsAssistantMessagesOnly() + { + ISessionManager session = await CreateSessionAsync(); + await session.AppendMessagesAsync( + [ + ConversationMessage.FromText(ConversationRole.User, "ask"), + ConversationMessage.FromText(ConversationRole.Assistant, "one") with + { + Usage = new MessageUsage(100, 40, 140) + }, + ConversationMessage.FromText(ConversationRole.Assistant, "no usage"), + ConversationMessage.FromText(ConversationRole.Assistant, "two") with + { + Usage = new MessageUsage(250, 60, 310) + }, + ], + CancellationToken.None); + + (long input, long output) = ActiveBranch.Load(session).SumAssistantUsage(); + + Assert.AreEqual(350, input); + Assert.AreEqual(100, output); + } + + [TestMethod] + public async Task SumAssistantUsageTreatsUnsetTokenCountsAsZero() + { + ISessionManager session = await CreateSessionAsync(); + await session.AppendMessagesAsync( + [ + ConversationMessage.FromText(ConversationRole.Assistant, "partial") with + { + Usage = new MessageUsage(InputTokens: 100) + }, + ], + CancellationToken.None); + + (long input, long output) = ActiveBranch.Load(session).SumAssistantUsage(); + + Assert.AreEqual(100, input); + Assert.AreEqual(0, output); + } + + [TestMethod] + public async Task FlattenMessagesPreservesBranchOrderAndContent() + { + ISessionManager session = await CreateSessionAsync(); + await session.AppendMessagesAsync( + [ + ConversationMessage.FromText(ConversationRole.User, "first"), + ConversationMessage.FromText(ConversationRole.Assistant, "second"), + ], + CancellationToken.None); + await session.AppendModelChangeAsync("local", "coder", CancellationToken.None); + await session.AppendMessagesAsync( + [ConversationMessage.FromText(ConversationRole.User, "third")], + CancellationToken.None); + + List messages = ActiveBranch.Load(session).FlattenMessages(); + + Assert.AreEqual(3, messages.Count); + Assert.AreEqual(ConversationRole.User, messages[0].Role); + Assert.AreEqual("first", messages[0].Text); + Assert.AreEqual(ConversationRole.Assistant, messages[1].Role); + Assert.AreEqual("second", messages[1].Text); + Assert.AreEqual(ConversationRole.User, messages[2].Role); + Assert.AreEqual("third", messages[2].Text); + } + + [TestMethod] + public async Task FromEntriesFlattensArbitraryEntryLists() + { + ISessionManager session = await CreateSessionAsync(); + await session.AppendMessagesAsync( + [ + ConversationMessage.FromText(ConversationRole.User, "one"), + ConversationMessage.FromText(ConversationRole.Assistant, "two"), + ], + CancellationToken.None); + IReadOnlyList entries = session.GetActiveBranch(); + + List messages = ActiveBranch.FromEntries(entries).FlattenMessages(); + + Assert.AreEqual(2, messages.Count); + Assert.AreEqual("two", messages[1].Text); + } + + [TestMethod] + public void SumMessageTextCharsTotalsTextLength() + { + ConversationState conversation = new(); + conversation.Add(ConversationMessage.FromText(ConversationRole.User, new string('x', 10))); + conversation.Add(ConversationMessage.FromText(ConversationRole.Assistant, new string('y', 25))); + + Assert.AreEqual(35, ActiveBranch.SumMessageTextChars(conversation)); + } + + private async Task CreateSessionAsync() + { + JsonlSessionStore store = new(_sessionsRoot); + SessionManagerFactory factory = new(store); + return await factory.CreateAsync(Environment.CurrentDirectory, CancellationToken.None); + } +} diff --git a/tests/WinHarness.IntegrationTests/ProviderConfiguratorTests.cs b/tests/WinHarness.IntegrationTests/ProviderConfiguratorTests.cs index ec28490..40345f1 100644 --- a/tests/WinHarness.IntegrationTests/ProviderConfiguratorTests.cs +++ b/tests/WinHarness.IntegrationTests/ProviderConfiguratorTests.cs @@ -136,6 +136,204 @@ await Assert.ThrowsExactlyAsync(async () => await configurator.AddProviderAsync("bad", "not-a-url", apiKey: null, makeDefault: false, CancellationToken.None)); } + [TestMethod] + public async Task SetDefaultProviderRepairsModelToFirstOfNewProvider() + { + ConfigStore store = new(_directory); + ProviderConfigurator configurator = new(store, new InMemoryCredentialStore()); + await SeedLocalAndHostedAsync(configurator); + + string resolvedModel = await configurator.SetDefaultProviderAsync("hosted", CancellationToken.None); + + Assert.AreEqual("gpt-primary", resolvedModel); + WinHarnessOptions saved = await store.LoadAsync(CancellationToken.None); + Assert.AreEqual("hosted", saved.DefaultProvider); + Assert.AreEqual("gpt-primary", saved.DefaultModel); + } + + [TestMethod] + public async Task SetDefaultProviderKeepsModelWhenStillValid() + { + ConfigStore store = new(_directory); + ProviderConfigurator configurator = new(store, new InMemoryCredentialStore()); + await SeedLocalAndHostedAsync(configurator); + await configurator.AddModelAsync( + "hosted", + "coder", + "qwen2.5-coder:latest", + ProviderCapabilities.None, + makeDefault: false, + contextWindow: null, + supportedReasoningEfforts: null, + cancellationToken: CancellationToken.None); + + // "coder" is hosted's second model; a repair would have picked "gpt-primary". + string resolvedModel = await configurator.SetDefaultProviderAsync("hosted", CancellationToken.None); + + Assert.AreEqual("coder", resolvedModel); + WinHarnessOptions saved = await store.LoadAsync(CancellationToken.None); + Assert.AreEqual("hosted", saved.DefaultProvider); + Assert.AreEqual("coder", saved.DefaultModel); + } + + [TestMethod] + public async Task SetDefaultProviderClearsModelWhenNewProviderHasNoModels() + { + ConfigStore store = new(_directory); + ProviderConfigurator configurator = new(store, new InMemoryCredentialStore()); + await SeedLocalAndHostedAsync(configurator); + await configurator.AddProviderAsync("empty", "https://empty.example.com/v1", apiKey: null, makeDefault: false, CancellationToken.None); + + string resolvedModel = await configurator.SetDefaultProviderAsync("empty", CancellationToken.None); + + Assert.AreEqual(string.Empty, resolvedModel); + WinHarnessOptions saved = await store.LoadAsync(CancellationToken.None); + Assert.AreEqual("empty", saved.DefaultProvider); + Assert.AreEqual(string.Empty, saved.DefaultModel); + } + + [TestMethod] + public async Task SetDefaultProviderThrowsForUnknownProvider() + { + ConfigStore store = new(_directory); + ProviderConfigurator configurator = new(store, new InMemoryCredentialStore()); + await SeedLocalAndHostedAsync(configurator); + + InvalidOperationException exception = await Assert.ThrowsExactlyAsync(async () => + await configurator.SetDefaultProviderAsync("missing", CancellationToken.None)); + Assert.AreEqual("Provider 'missing' is not configured.", exception.Message); + } + + [TestMethod] + public async Task SetDefaultModelSwitchesWithinDefaultProvider() + { + ConfigStore store = new(_directory); + ProviderConfigurator configurator = new(store, new InMemoryCredentialStore()); + await SeedLocalAndHostedAsync(configurator); + await configurator.AddModelAsync( + "local", + "reviewer", + "qwen2.5-coder:latest", + ProviderCapabilities.None, + makeDefault: false, + contextWindow: null, + supportedReasoningEfforts: null, + cancellationToken: CancellationToken.None); + + await configurator.SetDefaultModelAsync("reviewer", CancellationToken.None); + + WinHarnessOptions saved = await store.LoadAsync(CancellationToken.None); + Assert.AreEqual("local", saved.DefaultProvider); + Assert.AreEqual("reviewer", saved.DefaultModel); + } + + [TestMethod] + public async Task SetDefaultModelRejectsModelFromAnotherProvider() + { + ConfigStore store = new(_directory); + ProviderConfigurator configurator = new(store, new InMemoryCredentialStore()); + await SeedLocalAndHostedAsync(configurator); + + InvalidOperationException exception = await Assert.ThrowsExactlyAsync(async () => + await configurator.SetDefaultModelAsync("gpt-primary", CancellationToken.None)); + Assert.AreEqual("Model 'gpt-primary' is not configured for provider 'local'.", exception.Message); + } + + [TestMethod] + public async Task SetDefaultModelThrowsWithoutDefaultProvider() + { + ConfigStore store = new(_directory); + ProviderConfigurator configurator = new(store, new InMemoryCredentialStore()); + + InvalidOperationException exception = await Assert.ThrowsExactlyAsync(async () => + await configurator.SetDefaultModelAsync("coder", CancellationToken.None)); + Assert.AreEqual("Configure a default provider before selecting a model.", exception.Message); + } + + [TestMethod] + public void RepairDefaultModelKeepsModelThatBelongsToProvider() + { + WinHarnessOptions options = CreateRepairOptions(); + + Assert.AreEqual("gpt-primary", ProviderConfigurator.RepairDefaultModel(options, "hosted", "gpt-primary")); + } + + [TestMethod] + public void RepairDefaultModelFallsBackToFirstModel() + { + WinHarnessOptions options = CreateRepairOptions(); + + Assert.AreEqual("gpt-primary", ProviderConfigurator.RepairDefaultModel(options, "hosted", "coder")); + } + + [TestMethod] + public void RepairDefaultModelClearsModelWhenProviderHasNone() + { + WinHarnessOptions options = CreateRepairOptions(); + + Assert.AreEqual(string.Empty, ProviderConfigurator.RepairDefaultModel(options, "empty", "coder")); + } + + [TestMethod] + public void RepairDefaultModelLeavesEmptyModelUnset() + { + WinHarnessOptions options = CreateRepairOptions(); + + Assert.AreEqual(string.Empty, ProviderConfigurator.RepairDefaultModel(options, "hosted", string.Empty)); + } + + [TestMethod] + public void RepairDefaultModelThrowsForUnknownProvider() + { + WinHarnessOptions options = CreateRepairOptions(); + + InvalidOperationException exception = Assert.ThrowsExactly(() => + ProviderConfigurator.RepairDefaultModel(options, "missing", "coder")); + Assert.AreEqual("Provider 'missing' is not configured.", exception.Message); + } + + private static async Task SeedLocalAndHostedAsync(ProviderConfigurator configurator) + { + await configurator.AddProviderAsync("local", "http://localhost:11434/v1", apiKey: null, makeDefault: true, CancellationToken.None); + await configurator.AddModelAsync( + "local", + "coder", + "qwen2.5-coder:latest", + ProviderCapabilities.None, + makeDefault: true, + contextWindow: null, + supportedReasoningEfforts: null, + cancellationToken: CancellationToken.None); + await configurator.AddProviderAsync("hosted", "https://api.openai.com/v1", apiKey: null, makeDefault: false, CancellationToken.None); + await configurator.AddModelAsync( + "hosted", + "gpt-primary", + "gpt-4.1", + ProviderCapabilities.None, + makeDefault: false, + contextWindow: null, + supportedReasoningEfforts: null, + cancellationToken: CancellationToken.None); + } + + private static WinHarnessOptions CreateRepairOptions() + { + WinHarnessOptions options = new() + { + DefaultProvider = "local", + DefaultModel = "coder" + }; + ProviderOptions local = new() { Id = "local", Kind = "openai-compatible", BaseUrl = "http://localhost:11434/v1" }; + local.Models.Add(new ModelOptions { Id = "coder", ProviderModelId = "qwen2.5-coder:latest" }); + ProviderOptions hosted = new() { Id = "hosted", Kind = "openai-compatible", BaseUrl = "https://api.openai.com/v1" }; + hosted.Models.Add(new ModelOptions { Id = "gpt-primary", ProviderModelId = "gpt-4.1" }); + ProviderOptions empty = new() { Id = "empty", Kind = "openai-compatible", BaseUrl = "https://empty.example.com/v1" }; + options.Providers.Add(local); + options.Providers.Add(hosted); + options.Providers.Add(empty); + return options; + } + private sealed class InMemoryCredentialStore : ICredentialStore { public Dictionary Secrets { get; } = new(StringComparer.Ordinal); diff --git a/tests/WinHarness.IntegrationTests/TurnPumpTests.cs b/tests/WinHarness.IntegrationTests/TurnPumpTests.cs new file mode 100644 index 0000000..3d9a13f --- /dev/null +++ b/tests/WinHarness.IntegrationTests/TurnPumpTests.cs @@ -0,0 +1,261 @@ +using System.Runtime.CompilerServices; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using WinHarness.Cli.Chat; +using WinHarness.Conversation; +using WinHarness.Infrastructure.Sessions; +using WinHarness.Runtime; +using WinHarness.Sessions; + +namespace WinHarness.IntegrationTests; + +[TestClass] +public sealed class TurnPumpTests +{ + private string _sessionsRoot = null!; + + [TestInitialize] + public void SetUp() + { + _sessionsRoot = Path.Combine(Path.GetTempPath(), "WinHarnessTurnPump", Guid.NewGuid().ToString("N")); + } + + [TestCleanup] + public void TearDown() + { + if (Directory.Exists(_sessionsRoot)) + { + Directory.Delete(_sessionsRoot, recursive: true); + } + } + + [TestMethod] + public async Task SuccessfulTurnAppendsFullArtifactsOnce() + { + ChatSession session = await CreateSessionAsync(); + TurnArtifacts artifacts = new( + [ + ConversationMessage.FromText(ConversationRole.User, "hi"), + ConversationMessage.FromText(ConversationRole.Assistant, "hello"), + ]); + ScriptedRuntime runtime = new( + new AgentEvent(AgentEventKind.AssistantDelta, "hello"), + new AgentEvent(AgentEventKind.Completed, TurnPump.NormalCompletionMessage, artifacts)); + + TurnOutcome outcome = await RunPumpAsync(session, runtime); + + Assert.IsNull(outcome.FailureMessage); + Assert.IsFalse(outcome.AppendedPartialArtifacts); + Assert.AreSame(artifacts, outcome.AppendedArtifacts); + CollectionAssert.AreEqual( + new[] { "hi", "hello" }, + session.Conversation.Messages.Select(static message => message.Text).ToArray()); + } + + [TestMethod] + public async Task SuccessfulTurnCapturesUsageInOutcome() + { + ChatSession session = await CreateSessionAsync(); + MessageUsage usage = new(InputTokens: 12, OutputTokens: 34, TotalTokens: 46); + TurnArtifacts artifacts = new( + [ + ConversationMessage.FromText(ConversationRole.User, "hi"), + ConversationMessage.FromText(ConversationRole.Assistant, "hello") with { Usage = usage }, + ]); + ScriptedRuntime runtime = new( + new AgentEvent(AgentEventKind.Completed, TurnPump.NormalCompletionMessage, artifacts)); + + TurnOutcome outcome = await RunPumpAsync(session, runtime); + + Assert.AreSame(usage, outcome.Usage); + } + + [TestMethod] + public async Task FailedThenPartialCompletionAppendsPartialArtifactsOnce() + { + ChatSession session = await CreateSessionAsync(); + TurnArtifacts partial = new( + [ + ConversationMessage.FromText(ConversationRole.User, "hi"), + ConversationMessage.FromText(ConversationRole.Assistant, "truncated"), + ]); + ScriptedRuntime runtime = new( + new AgentEvent(AgentEventKind.AssistantDelta, "trunc"), + new AgentEvent(AgentEventKind.Failed, "boom"), + new AgentEvent(AgentEventKind.Completed, TurnPump.PartialCompletionMessage, partial)); + + List observed = []; + TurnPump pump = new(session); + TurnOutcome outcome = await pump.RunAsync( + CreateRequest(session), + runtime, + agentEvent => + { + observed.Add(agentEvent.Kind); + return ValueTask.CompletedTask; + }, + CancellationToken.None); + + Assert.AreEqual("boom", outcome.FailureMessage); + Assert.IsTrue(outcome.AppendedPartialArtifacts); + Assert.AreSame(partial, outcome.AppendedArtifacts); + CollectionAssert.AreEqual( + new[] { "hi", "truncated" }, + session.Conversation.Messages.Select(static message => message.Text).ToArray()); + CollectionAssert.AreEqual( + new[] { AgentEventKind.AssistantDelta, AgentEventKind.Failed, AgentEventKind.Completed }, + observed); + } + + [TestMethod] + public async Task FailureWithoutPartialCompletionAppendsNothing() + { + ChatSession session = await CreateSessionAsync(); + ScriptedRuntime runtime = new( + new AgentEvent(AgentEventKind.AssistantDelta, "partial text"), + new AgentEvent(AgentEventKind.Failed, "provider exploded")); + + TurnOutcome outcome = await RunPumpAsync(session, runtime); + + Assert.AreEqual("provider exploded", outcome.FailureMessage); + Assert.IsNull(outcome.AppendedArtifacts); + Assert.IsFalse(outcome.AppendedPartialArtifacts); + Assert.AreEqual(0, session.Conversation.Messages.Count); + } + + [TestMethod] + public async Task PresentObservesEventsInStreamOrder() + { + ChatSession session = await CreateSessionAsync(); + ScriptedRuntime runtime = new( + new AgentEvent(AgentEventKind.AssistantDelta, "a"), + new AgentEvent( + AgentEventKind.ToolActivity, + "tool", + ToolActivity: new ToolActivityInfo("read_file", ToolActivityPhase.Started)), + new AgentEvent(AgentEventKind.AssistantDelta, "b"), + new AgentEvent( + AgentEventKind.Completed, + TurnPump.NormalCompletionMessage, + new TurnArtifacts([ConversationMessage.FromText(ConversationRole.Assistant, "ab")]))); + + List<(AgentEventKind Kind, string Message)> observed = []; + TurnPump pump = new(session); + await pump.RunAsync( + CreateRequest(session), + runtime, + agentEvent => + { + observed.Add((agentEvent.Kind, agentEvent.Message)); + return ValueTask.CompletedTask; + }, + CancellationToken.None); + + CollectionAssert.AreEqual( + new[] + { + (AgentEventKind.AssistantDelta, "a"), + (AgentEventKind.ToolActivity, "tool"), + (AgentEventKind.AssistantDelta, "b"), + (AgentEventKind.Completed, TurnPump.NormalCompletionMessage), + }, + observed); + } + + [TestMethod] + public async Task UnconsumedSteeringIsPromotedToFollowUps() + { + ChatSession session = await CreateSessionAsync(); + session.Steering.Enqueue("keep going"); + ScriptedRuntime runtime = new( + new AgentEvent( + AgentEventKind.Completed, + TurnPump.NormalCompletionMessage, + new TurnArtifacts([ConversationMessage.FromText(ConversationRole.Assistant, "done")]))); + + await RunPumpAsync(session, runtime); + + Queue followUps = new(); + TurnPump.PromoteUnconsumedSteering(session.Steering, followUps); + + Assert.AreEqual(0, session.Steering.Count); + CollectionAssert.AreEqual(new[] { "keep going" }, followUps.ToArray()); + } + + [TestMethod] + public async Task CancellationPropagatesAndAppendsNothing() + { + ChatSession session = await CreateSessionAsync(); + CancellingRuntime runtime = new(); + + await Assert.ThrowsExactlyAsync(async () => + await RunPumpAsync(session, runtime)); + + Assert.AreEqual(0, session.Conversation.Messages.Count); + } + + private async Task CreateSessionAsync() + { + JsonlSessionStore store = new(_sessionsRoot); + SessionManagerFactory factory = new(store); + ISessionManager sessionManager = await factory.CreateAsync(Environment.CurrentDirectory, CancellationToken.None); + return new ChatSession(sessionManager, "local", "coder", renderMarkdown: false); + } + + private static AgentRunRequest CreateRequest(ChatSession session) => + new( + session.ProviderId, + session.ModelId, + session.CreateRunConversation("hi"), + session.WorkspaceRoot, + session.ProjectContext, + session.ReasoningEffort, + session.ToolFilter, + session.Steering); + + private static async ValueTask RunPumpAsync(ChatSession session, IAgentRuntime runtime) + { + TurnPump pump = new(session); + return await pump.RunAsync( + CreateRequest(session), + runtime, + static _ => ValueTask.CompletedTask, + CancellationToken.None); + } + + private sealed class ScriptedRuntime : IAgentRuntime + { + private readonly IReadOnlyList _events; + + public ScriptedRuntime(params AgentEvent[] events) + { + _events = events; + } + + public async IAsyncEnumerable RunAsync( + AgentRunRequest request, + [EnumeratorCancellation] CancellationToken cancellationToken) + { + _ = request; + foreach (AgentEvent agentEvent in _events) + { + cancellationToken.ThrowIfCancellationRequested(); + yield return agentEvent; + } + + await Task.CompletedTask; + } + } + + private sealed class CancellingRuntime : IAgentRuntime + { + public async IAsyncEnumerable RunAsync( + AgentRunRequest request, + [EnumeratorCancellation] CancellationToken cancellationToken) + { + _ = request; + yield return new AgentEvent(AgentEventKind.AssistantDelta, "partial"); + await Task.CompletedTask; + throw new OperationCanceledException(); + } + } +}