From 8ddc98e958c4d8af759529f79445a4e0fa8bfae5 Mon Sep 17 00:00:00 2001 From: Aaron Stannard Date: Tue, 11 Aug 2026 21:41:54 -0500 Subject: [PATCH] fix(cli): route daemon protocol diagnostics away from json stdout The report named tool_activity as an unknown daemon output type. This branch defines no such type. No session output type maps to it. No code path sends it to a client. The daemon emitter does not exist here. The headless --json channel already sent ErrorOutput text to stderr. It did not send that text to stdout. Stdout held only the JSON envelope before this change. This change adds a direct safeguard. The mapper now marks an unknown output type as a protocol diagnostic. A protocol diagnostic is not a session error. HeadlessChannel sends a protocol diagnostic to stderr, the session log, and the logger. A real session error keeps its prior path. Stdout in --json mode never carries a diagnostic line. Tests: - DaemonClientMappingTests: an unknown output type sets IsProtocolDiagnostic to true. A real daemon error keeps the flag false. - HeadlessChannelJsonStdoutHygieneTests: an unknown output type flows through a full headless turn. Stdout holds one parseable JSON envelope. Stderr and the logger both record the diagnostic. --- .../Protocol/SessionOutputDtoMapper.cs | 6 +- .../Sessions/SessionProtocol.Outputs.cs | 10 ++ .../Cli/DaemonClientMappingTests.cs | 37 +++++ .../HeadlessChannelJsonStdoutHygieneTests.cs | 152 ++++++++++++++++++ src/Netclaw.Cli/HeadlessChannel.cs | 11 ++ 5 files changed, 215 insertions(+), 1 deletion(-) create mode 100644 src/Netclaw.Cli.Tests/Cli/HeadlessChannelJsonStdoutHygieneTests.cs diff --git a/src/Netclaw.Actors/Protocol/SessionOutputDtoMapper.cs b/src/Netclaw.Actors/Protocol/SessionOutputDtoMapper.cs index 0a60f7565..d79ce6856 100644 --- a/src/Netclaw.Actors/Protocol/SessionOutputDtoMapper.cs +++ b/src/Netclaw.Actors/Protocol/SessionOutputDtoMapper.cs @@ -349,11 +349,15 @@ public static SessionOutput FromDto(SessionOutputDto dto) IsMessy = dto.InteractionIsMessy ?? false, Options = dto.InteractionOptions ?? [] }, + // The client build does not know dto.Type. This is a protocol + // read failure, not a session error — flag it so a channel + // adapter keeps it off any machine-readable result envelope. _ => new ErrorOutput { SessionId = sessionId, TimestampMs = dto.TimestampMs, - Message = $"Unknown output type from daemon: {dto.Type}" + Message = $"Unknown output type from daemon: {dto.Type}", + IsProtocolDiagnostic = true } }; } diff --git a/src/Netclaw.Actors/Sessions/SessionProtocol.Outputs.cs b/src/Netclaw.Actors/Sessions/SessionProtocol.Outputs.cs index 8fa0e4612..30fd69f88 100644 --- a/src/Netclaw.Actors/Sessions/SessionProtocol.Outputs.cs +++ b/src/Netclaw.Actors/Sessions/SessionProtocol.Outputs.cs @@ -226,6 +226,16 @@ public sealed record ErrorOutput : SessionOutput /// for diagnostic logging by subscribers and adapters. /// public Exception? Cause { get; init; } + + /// + /// True when this instance is not a real session error. The client + /// mapper sets it when it cannot read a wire message from the daemon + /// (for example, an output type the client build does not know). + /// A channel adapter must keep a protocol diagnostic out of any + /// machine-readable result envelope and must not report it as a turn + /// error. Defaults to false for every daemon-originated error. + /// + public bool IsProtocolDiagnostic { get; init; } } /// diff --git a/src/Netclaw.Cli.Tests/Cli/DaemonClientMappingTests.cs b/src/Netclaw.Cli.Tests/Cli/DaemonClientMappingTests.cs index 2bf5926ee..f201c046f 100644 --- a/src/Netclaw.Cli.Tests/Cli/DaemonClientMappingTests.cs +++ b/src/Netclaw.Cli.Tests/Cli/DaemonClientMappingTests.cs @@ -91,6 +91,43 @@ public void FromDto_unknown_type_becomes_error_output() Assert.Equal("signalr/test", error.SessionId.Value); } + [Fact] + public void FromDto_unknown_type_flags_a_protocol_diagnostic_not_a_session_error() + { + // A daemon and CLI on different builds can disagree on the output + // type set (for example a newer daemon that streams "tool_activity"). + // The mapper must mark this a client read failure, not session + // output, so a channel adapter never reports it as a turn error. + var dto = new SessionOutputDto + { + Type = "tool_activity", + SessionId = "signalr/test", + TimestampMs = 123 + }; + + var output = DaemonClient.FromDto(dto); + + var error = Assert.IsType(output); + Assert.True(error.IsProtocolDiagnostic); + } + + [Fact] + public void FromDto_daemon_error_output_is_not_a_protocol_diagnostic() + { + var dto = new SessionOutputDto + { + Type = "error", + SessionId = "signalr/test", + TimestampMs = 123, + ErrorMessage = "The provider returned a 500." + }; + + var output = DaemonClient.FromDto(dto); + + var error = Assert.IsType(output); + Assert.False(error.IsProtocolDiagnostic); + } + [Fact] public void FromDto_maps_session_joined_with_recent_messages() { diff --git a/src/Netclaw.Cli.Tests/Cli/HeadlessChannelJsonStdoutHygieneTests.cs b/src/Netclaw.Cli.Tests/Cli/HeadlessChannelJsonStdoutHygieneTests.cs new file mode 100644 index 000000000..d999c6ee6 --- /dev/null +++ b/src/Netclaw.Cli.Tests/Cli/HeadlessChannelJsonStdoutHygieneTests.cs @@ -0,0 +1,152 @@ +// ----------------------------------------------------------------------- +// +// Copyright (C) 2026 - 2026 Petabridge, LLC +// +// ----------------------------------------------------------------------- +using System.Text.Json; +using Microsoft.Extensions.Hosting; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Time.Testing; +using Netclaw.Actors.Protocol; +using Netclaw.Cli.Daemon; +using Netclaw.Configuration; +using Netclaw.Tests.Utilities; +using Xunit; + +namespace Netclaw.Cli.Tests.Cli; + +/// +/// Proves that chat -p --json keeps stdout pure JSON when the daemon +/// sends an output type this CLI build does not recognize (for example a +/// newer daemon streaming "tool_activity" to an older client). The client +/// mapper turns the unrecognized type into a diagnostic ErrorOutput +/// (); this suite proves the +/// headless channel keeps that diagnostic off stdout, still surfaces it +/// (stderr + logger), and still emits a parseable JSON envelope. +/// +[Collection("Update verification")] +public sealed class HeadlessChannelJsonStdoutHygieneTests : IDisposable +{ + private static readonly TimeSpan[] ImmediateDelays = [TimeSpan.Zero]; + + private readonly DisposableTempDir _dir = new(); + private readonly TextWriter _originalOut = Console.Out; + private readonly TextWriter _originalError = Console.Error; + + public void Dispose() + { + Console.SetOut(_originalOut); + Console.SetError(_originalError); + _dir.Dispose(); + } + + [Fact] + public async Task Unrecognized_output_type_stays_off_stdout_and_envelope_stays_parseable() + { + var transport = new FakeDaemonHubTransport(); + + // Model a daemon that streams an output type this CLI build does + // not know about (e.g. "tool_activity") ahead of the real reply. + transport.VoidInvokeHook = (method, args, _) => + { + if (method == "SendMessage") + { + var sessionId = (string)args[0]!; + transport.PushOutput(new SessionOutputDto + { + Type = "tool_activity", + SessionId = sessionId, + TimestampMs = 1 + }); + transport.PushOutput(new SessionOutputDto + { + Type = "text", + SessionId = sessionId, + TimestampMs = 2, + Text = "hello" + }); + transport.PushOutput(new SessionOutputDto + { + Type = "turn_completed", + SessionId = sessionId, + TimestampMs = 3, + TurnNumber = new TurnNumber(1) + }); + } + + return Task.CompletedTask; + }; + + await using var daemonClient = new DaemonClient( + "http://localhost", + transport, + reconnectDelays: ImmediateDelays, + rpcTimeout: TimeSpan.FromSeconds(5)); + + var paths = new NetclawPaths(_dir.Path); + var lifetime = new RecordingHostLifetime(); + var logger = new RecordingLogger(); + var options = new HeadlessOptions("hi") { JsonOutput = true }; + + var channel = new HeadlessChannel( + daemonClient, paths, lifetime, new FakeTimeProvider(), options, logger); + + var stdout = new StringWriter(); + var stderr = new StringWriter(); + Console.SetOut(stdout); + Console.SetError(stderr); + + await channel.StartAsync(TestContext.Current.CancellationToken); + await lifetime.StopRequested.Task.WaitAsync(TimeSpan.FromSeconds(5), TestContext.Current.CancellationToken); + + var stdoutText = stdout.ToString(); + var stderrText = stderr.ToString(); + + // Stdout carries exactly the JSON envelope — nothing else. + var envelope = JsonSerializer.Deserialize(stdoutText.Trim()); + Assert.Equal("hello", envelope.GetProperty("response").GetString()); + Assert.DoesNotContain("Unknown output type", stdoutText, StringComparison.Ordinal); + Assert.DoesNotContain("[diagnostic]", stdoutText, StringComparison.Ordinal); + Assert.DoesNotContain("[error]", stdoutText, StringComparison.Ordinal); + + // The diagnostic is not silently dropped — stderr and the logger both see it. + Assert.Contains("Unknown output type from daemon: tool_activity", stderrText, StringComparison.Ordinal); + Assert.Contains(logger.Messages, m => m.Contains("tool_activity", StringComparison.Ordinal)); + Assert.Contains(logger.Levels, l => l == LogLevel.Warning); + } + + private sealed class RecordingHostLifetime : IHostApplicationLifetime + { + public TaskCompletionSource StopRequested { get; } = new(TaskCreationOptions.RunContinuationsAsynchronously); + + public CancellationToken ApplicationStarted => CancellationToken.None; + + public CancellationToken ApplicationStopping { get; } = CancellationToken.None; + + public CancellationToken ApplicationStopped => CancellationToken.None; + + public void StopApplication() => StopRequested.TrySetResult(); + } + + private sealed class RecordingLogger : ILogger + { + public List Messages { get; } = []; + + public List Levels { get; } = []; + + public IDisposable? BeginScope(TState state) where TState : notnull => null; + + public bool IsEnabled(LogLevel logLevel) => true; + + public void Log( + LogLevel logLevel, + EventId eventId, + TState state, + Exception? exception, + Func formatter) + { + Levels.Add(logLevel); + Messages.Add(formatter(state, exception)); + } + } +} diff --git a/src/Netclaw.Cli/HeadlessChannel.cs b/src/Netclaw.Cli/HeadlessChannel.cs index 5e80a2885..9f1d373a9 100644 --- a/src/Netclaw.Cli/HeadlessChannel.cs +++ b/src/Netclaw.Cli/HeadlessChannel.cs @@ -269,6 +269,17 @@ private void HandleOutput(SessionOutput output, StreamWriter? log) Log(log, $"USAGE: in={msg.InputTokens} out={msg.OutputTokens} total={msg.TotalTokens} cached={msg.CachedInputTokens} reasoning={msg.ReasoningTokens} context_window={msg.ContextWindowTokens} prompt_ms={msg.PromptMs} predicted_tok_s={msg.PredictedPerSecond}"); break; + case ErrorOutput { IsProtocolDiagnostic: true } msg: + // The client could not read a daemon wire message (unknown + // output type). This is not a turn result, so it must never + // reach stdout — json mode stdout carries only the envelope. + // It still must not go silent: stderr, the session log, and + // the structured logger all see it. + _logger.LogWarning("Daemon protocol diagnostic: {Message}", msg.Message); + Console.Error.WriteLine($"[diagnostic] {msg.Message}"); + Log(log, $"DIAGNOSTIC: {msg.Message}"); + break; + case ErrorOutput msg: Console.Error.WriteLine($"[error] {msg.Message}"); Log(log, $"ERROR: {msg.Message}");