From 426f8a8f810d1200db3b95ac2c3de20648eb335a Mon Sep 17 00:00:00 2001 From: David Perez Date: Sun, 12 Jul 2026 14:57:50 -0500 Subject: [PATCH] fix: report unfinished tool batches accurately Distinguish interim and terminal batch settlement so unfinished tools are shown as running while a turn can still receive completion events, and interrupted once the turn has ended or failed. Document the accepted safe display-label scope and terminal rendering terms added by compact tool output. --- CONTEXT.md | 20 ++++++++ README.md | 2 +- src/WinHarness.Cli/Program.cs | 6 +-- .../Rendering/ToolBatchRenderer.cs | 48 +++++++++++++------ .../Runtime/SingleAgentRuntime.cs | 2 + src/WinHarness.Tools/IToolActivitySink.cs | 7 +-- .../ToolBatchRendererTests.cs | 22 +++++++-- 7 files changed, 82 insertions(+), 25 deletions(-) diff --git a/CONTEXT.md b/CONTEXT.md index 59dae7d..f60fe81 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -49,6 +49,26 @@ _Avoid_: Engine, deployment. A provider-independent capability the model can invoke (built-in file/command tools or MCP stdio tools), exposed through one tool interface. _Avoid_: Function (reserved for the provider-transport AIFunction), plugin, command. +**Tool round-trip**: +One model-requested Tool invocation and its result within a Turn. The model may perform several tool round-trips before producing final assistant text. +_Avoid_: Function call (provider transport detail), command (only one kind of Tool). + +**Tool batch**: +A group of adjacent tool-activity events rendered together by the interactive chat UI. A batch is presentation-only; it does not change Turn artifacts or provider-visible tool round-trips. +_Avoid_: Turn, transaction. + +**Tool run**: +The terminal UI's count label for one observed Tool execution inside a compact Tool batch. Prefer tool round-trip when discussing runtime/domain behavior, and tool run only for user-facing compact-renderer copy. An unfinished tool run is reported as `running` during an interim flush and `interrupted` when the Turn has ended. +_Avoid_: Tool call (ambiguous with provider transport and message schema). + +**Display label**: +A short, safe-to-print Tool invocation label for terminal output. It may include structured file paths, but must not include arbitrary command/search text or secrets, and is not part of the JSON event stream. +_Avoid_: Arguments, payload. + +**Verbose tool rendering**: +Interactive chat mode that prints one persistent line per tool-activity event instead of compact Tool batch summaries. Enabled by `winharness chat --verbose` for debugging output flow. +_Avoid_: Debug mode (broader meaning). + **Tool filter**: An optional per-run gating policy over Tools by raw name: allowlist, denylist, or disable-all. Applied when building the model-facing tool list for a Turn; does not affect `tools call` or discovery. _Avoid_: Permissions, sandbox (different concerns). diff --git a/README.md b/README.md index 0792e95..64d38e5 100644 --- a/README.md +++ b/README.md @@ -62,7 +62,7 @@ dotnet publish .\src\WinHarness.Cli\WinHarness.Cli.csproj -c Release -r win-x64 - `winharness chat --prompt "..." --output json` — emit LF-delimited JSONL events (turn/tool/usage/error) on stdout for scripting; see `docs/design/json-events.md` - `winharness rpc` — long-lived process integration over stdin/stdout JSON (prompt, steer, abort, session ops); see `docs/design/rpc.md` and `samples/rpc-client.ps1` - `winharness chat` for the terminal REPL (continues the most recent workspace session by default; see [Sessions](#sessions)) -- `winharness chat --verbose` to show one persistent line per tool event instead of compact batch summaries +- `winharness chat --verbose` to show one persistent line per tool event instead of compact batch summaries; compact mode uses short, safe display labels for supported file tools and never prints arbitrary command/search arguments - `winharness chat --tools read_file,grep,glob` to allowlist tools for the run; `--exclude-tools run_command` to deny specific tools; `--no-tools` to disable all tools (applies to built-in and MCP tools; unknown names warn instead of failing) - `winharness providers list` - `winharness providers add --id openai-main --base-url https://api.openai.com/v1 [--api-key sk-... --set-default]` diff --git a/src/WinHarness.Cli/Program.cs b/src/WinHarness.Cli/Program.cs index 12fa8a0..8f7c15d 100644 --- a/src/WinHarness.Cli/Program.cs +++ b/src/WinHarness.Cli/Program.cs @@ -1886,7 +1886,7 @@ private static async ValueTask RunTurnAsync( ChatSession session, string prompt, CancellationToken cancellationToken, - bool verbose = false) + bool verbose) { IAgentRuntime runtime = services.GetRequiredService(); Conversation runConversation = session.CreateRunConversation(prompt); @@ -2008,7 +2008,7 @@ async ValueTask FinalizeSegmentAsync() { await FinalizeSegmentAsync().ConfigureAwait(false); await thinking.StopAsync().ConfigureAwait(false); - toolBatch.Settle(); + toolBatch.Settle(terminal: true); } AnsiConsole.MarkupLine("[red]" + Markup.Escape(agentEvent.Message) + "[/]"); @@ -2082,7 +2082,7 @@ await session.AppendTurnAsync(agentEvent.TurnArtifacts, cancellationToken) if (interactive) { await FinalizeSegmentAsync().ConfigureAwait(false); - toolBatch.Settle(); + toolBatch.Settle(terminal: true); // The provider can close the stream with no text and no failure (e.g. an // empty completion or a response that was entirely reasoning tokens). Without diff --git a/src/WinHarness.Cli/Rendering/ToolBatchRenderer.cs b/src/WinHarness.Cli/Rendering/ToolBatchRenderer.cs index d650f86..b9bd618 100644 --- a/src/WinHarness.Cli/Rendering/ToolBatchRenderer.cs +++ b/src/WinHarness.Cli/Rendering/ToolBatchRenderer.cs @@ -13,6 +13,11 @@ namespace WinHarness.Cli.Rendering; /// internal sealed class ToolBatchRenderer { + private const string PendingIcon = "[dim]⠋[/]"; + private const string SuccessIcon = "[green]✓[/]"; + private const string FailedIcon = "[red]✗[/]"; + private const string MixedIcon = "[yellow]~[/]"; + private readonly bool _verbose; private readonly IAnsiConsole _console; private int _active; @@ -106,10 +111,15 @@ public void OnEvent(ToolActivityInfo info) /// /// Writes one summary line for the in-flight batch and clears counters. - /// No-op when no batch is pending. Called between assistant text - /// segments or at the end of a turn. + /// No-op when no batch is pending. /// - public void Settle() + /// + /// true when the turn has ended or failed, so unfinished tools can no + /// longer receive completion events and are reported as interrupted. + /// false for an interim flush between assistant-text segments, where + /// unfinished tools are still executing and are reported as running. + /// + public void Settle(bool terminal = false) { if (!HasPendingBatch) { @@ -118,7 +128,13 @@ public void Settle() int calls = _calls + _active; int ok = _ok; - int failed = _failed + _active; + int failed = _failed; + // Tools still executing when the batch settles are never folded into the + // failed count — conflating not-yet-finished with failed is user-visible + // misinformation, and they have contributed no duration yet. On an interim + // flush they are still live ("running"); at terminal settlement they can + // no longer complete, so they are "interrupted". + int unfinished = _active; TimeSpan duration = _duration; _active = 0; _calls = 0; @@ -131,8 +147,12 @@ public void Settle() ? "[bold]tool run[/]" : $"[bold]{calls} tool runs[/]"; + string unfinishedText = unfinished > 0 + ? $" · {unfinished} {(terminal ? "interrupted" : "running")}" + : string.Empty; + _console.MarkupLine( - $"{IconFor(ok, failed)} {header} [dim]· {ok} ok · {failed} failed · {durationText}[/]"); + $"{IconFor(ok, failed, unfinished)} {header} [dim]· {ok} ok · {failed} failed{unfinishedText} · {durationText}[/]"); } /// @@ -146,12 +166,12 @@ private void RenderVerboseLine(ToolActivityInfo info) switch (info.Phase) { case ToolActivityPhase.Started: - _console.MarkupLine($"[dim]⠋[/] [bold]{label}[/]"); + _console.MarkupLine($"{PendingIcon} [bold]{label}[/]"); break; case ToolActivityPhase.Completed: { - string icon = info.Succeeded == false ? "[red]✗[/]" : "[green]✓[/]"; + string icon = info.Succeeded == false ? FailedIcon : SuccessIcon; string duration = FormatDuration(info.Duration); _console.MarkupLine($"{icon} [bold]{label}[/] [dim]({duration})[/]"); break; @@ -163,25 +183,25 @@ private void RenderVerboseLine(ToolActivityInfo info) string exc = info.ExceptionTypeName is null ? "" : $" [red]{Markup.Escape(info.ExceptionTypeName)}[/]"; - _console.MarkupLine($"[red]✗[/] [bold]{label}[/] [dim]({duration})[/]{exc}"); + _console.MarkupLine($"{FailedIcon} [bold]{label}[/] [dim]({duration})[/]{exc}"); break; } } } - private static string IconFor(int ok, int failed) + private static string IconFor(int ok, int failed, int running) { - if (failed == 0) + if (failed == 0 && running == 0) { - return "[green]✓[/]"; + return SuccessIcon; } - if (ok == 0) + if (failed > 0 && ok == 0 && running == 0) { - return "[red]✗[/]"; + return FailedIcon; } - return "[yellow]~[/]"; + return MixedIcon; } private static string FormatDuration(TimeSpan? duration) diff --git a/src/WinHarness.Core/Runtime/SingleAgentRuntime.cs b/src/WinHarness.Core/Runtime/SingleAgentRuntime.cs index 71ec565..5528fd7 100644 --- a/src/WinHarness.Core/Runtime/SingleAgentRuntime.cs +++ b/src/WinHarness.Core/Runtime/SingleAgentRuntime.cs @@ -295,6 +295,8 @@ await WriteProviderDiagnosticAsync( messageUsage)); } + // Materializes the ValueTask so the streaming loop can race provider output + // against tool-activity notifications with Task.WhenAny. private static async Task MoveNextAsync(IAsyncEnumerator updates) { return await updates.MoveNextAsync().ConfigureAwait(false); diff --git a/src/WinHarness.Tools/IToolActivitySink.cs b/src/WinHarness.Tools/IToolActivitySink.cs index ba6fbdf..4422f4d 100644 --- a/src/WinHarness.Tools/IToolActivitySink.cs +++ b/src/WinHarness.Tools/IToolActivitySink.cs @@ -7,9 +7,10 @@ public interface IToolActivitySink { /// /// Records that a tool started. is a short, - /// safe-to-print summary of the invocation (e.g. "run_command Get-Command - /// firecrawl"); it may be null when the arguments are not available - /// or when no per-tool summarizer is registered. + /// safe-to-print summary of the invocation (for example, "read_file + /// README.md"); it may be null when the arguments are not available + /// or when no per-tool summarizer is registered. Labels must not expose + /// arbitrary command/search text or secrets. /// void ToolStarted(string toolName, string? displayLabel); diff --git a/tests/WinHarness.IntegrationTests/ToolBatchRendererTests.cs b/tests/WinHarness.IntegrationTests/ToolBatchRendererTests.cs index 7654d06..fbc46f8 100644 --- a/tests/WinHarness.IntegrationTests/ToolBatchRendererTests.cs +++ b/tests/WinHarness.IntegrationTests/ToolBatchRendererTests.cs @@ -59,11 +59,11 @@ public void SettlementCountsCompletedAndUnfinishedTools() renderer.Settle(); - StringAssert.Contains(PlainText(output), "2 tool runs · 1 ok · 1 failed"); + StringAssert.Contains(PlainText(output), "2 tool runs · 1 ok · 0 failed · 1 running"); } [TestMethod] - public void SettlementMarksUnfinishedToolAsFailed() + public void InterimSettlementMarksUnfinishedToolAsRunning() { using StringWriter output = new(); ToolBatchRenderer renderer = new(verbose: false, console: CreateConsole(output)); @@ -72,8 +72,22 @@ public void SettlementMarksUnfinishedToolAsFailed() renderer.Settle(); string rendered = PlainText(output); - StringAssert.Contains(rendered, "tool run · 0 ok · 1 failed"); - Assert.IsFalse(rendered.Contains("0 ok · 0 failed", StringComparison.Ordinal)); + StringAssert.Contains(rendered, "tool run · 0 ok · 0 failed · 1 running"); + Assert.IsFalse(rendered.Contains("0 ok · 1 failed", StringComparison.Ordinal)); + } + + [TestMethod] + public void TerminalSettlementMarksUnfinishedToolAsInterrupted() + { + using StringWriter output = new(); + ToolBatchRenderer renderer = new(verbose: false, console: CreateConsole(output)); + renderer.OnEvent(new ToolActivityInfo("run_command", ToolActivityPhase.Started)); + + renderer.Settle(terminal: true); + + string rendered = PlainText(output); + StringAssert.Contains(rendered, "tool run · 0 ok · 0 failed · 1 interrupted"); + Assert.IsFalse(rendered.Contains("0 ok · 1 failed", StringComparison.Ordinal)); } private static IAnsiConsole CreateConsole(TextWriter output)