diff --git a/SalmonEgg/SalmonEgg/Controls/ToolCallPill.xaml b/SalmonEgg/SalmonEgg/Controls/ToolCallPill.xaml index 33191fac..741f5a60 100644 --- a/SalmonEgg/SalmonEgg/Controls/ToolCallPill.xaml +++ b/SalmonEgg/SalmonEgg/Controls/ToolCallPill.xaml @@ -337,6 +337,15 @@ FontWeight="Normal" Foreground="{ThemeResource TextFillColorSecondaryBrush}" TextTrimming="CharacterEllipsis" /> + + - + + + + + diff --git a/SalmonEgg/SalmonEgg/Controls/ToolCallPill.xaml.cs b/SalmonEgg/SalmonEgg/Controls/ToolCallPill.xaml.cs index 331dc2a6..7db19950 100644 --- a/SalmonEgg/SalmonEgg/Controls/ToolCallPill.xaml.cs +++ b/SalmonEgg/SalmonEgg/Controls/ToolCallPill.xaml.cs @@ -32,6 +32,18 @@ public sealed partial class ToolCallPill : UserControl, INotifyPropertyChanged typeof(ToolCallPill), new PropertyMetadata(null, OnDisplayInputChanged)); + /// + /// The agent's programmatic name for the invoked tool (for example read_file). Shown as a + /// secondary label under the title, per the ACP Tool Call Name RFD's guidance to keep + /// primary. Optional: agents may omit it. + /// + public static readonly DependencyProperty ToolNameProperty = + DependencyProperty.Register( + nameof(ToolName), + typeof(string), + typeof(ToolCallPill), + new PropertyMetadata(string.Empty, OnDisplayInputChanged)); + public static readonly DependencyProperty StatusProperty = DependencyProperty.Register(nameof(Status), typeof(ToolCallStatus?), typeof(ToolCallPill), new PropertyMetadata(null, OnDisplayInputChanged)); @@ -102,6 +114,12 @@ public ToolCallKind? ToolKind set => SetValue(ToolKindProperty, value); } + public string ToolName + { + get => (string)GetValue(ToolNameProperty); + set => SetValue(ToolNameProperty, value); + } + public ToolCallStatus? Status { get => (ToolCallStatus?)GetValue(StatusProperty); @@ -184,10 +202,14 @@ public ICommand? ReportCommand public bool HasSummary => !string.IsNullOrWhiteSpace(Summary); + public bool HasToolName => !string.IsNullOrWhiteSpace(ToolName); + public bool HasDisplayItems => DetailItems?.Count > 0; public bool HasPendingPermissionRequest => PendingPermissionRequest != null; + public bool HasPermissionToolName => PendingPermissionRequest?.HasToolCallName == true; + public IReadOnlyList PermissionOptions { get @@ -211,8 +233,20 @@ public string AutomationName { get { - var name = DisplayToolName; - return HasSummary ? $"{name}, {Summary}" : name; + // The programmatic tool name is part of what the row says, so a screen reader has to reach + // it too: it is the only place the user learns read_file and grep are different tools. + var parts = new List(3) { DisplayToolName }; + if (HasToolName) + { + parts.Add(ToolName); + } + + if (HasSummary) + { + parts.Add(Summary); + } + + return string.Join(", ", parts); } } @@ -228,6 +262,7 @@ public ToolCallPill() NotifyDisplayChanged(); OnPropertyChanged(nameof(HasPendingPermissionRequest)); OnPropertyChanged(nameof(PermissionOptions)); + OnPropertyChanged(nameof(HasPermissionToolName)); DataContextChanged += ToolCallPill_DataContextChanged; Loaded += ToolCallPill_Loaded; } @@ -257,6 +292,7 @@ private static void OnPermissionInputChanged(DependencyObject d, DependencyPrope { pill.OnPropertyChanged(nameof(HasPendingPermissionRequest)); pill.OnPropertyChanged(nameof(PermissionOptions)); + pill.OnPropertyChanged(nameof(HasPermissionToolName)); } } @@ -282,6 +318,7 @@ private static void OnVisualStateInputChanged(DependencyObject d, DependencyProp private void NotifyDisplayChanged() { OnPropertyChanged(nameof(DisplayToolName)); + OnPropertyChanged(nameof(HasToolName)); OnPropertyChanged(nameof(HasSummary)); OnPropertyChanged(nameof(HasDisplayItems)); OnPropertyChanged(nameof(HasRawInput)); diff --git a/SalmonEgg/SalmonEgg/Styles/ChatStyles.xaml b/SalmonEgg/SalmonEgg/Styles/ChatStyles.xaml index d1c8dcdf..3f8f924a 100644 --- a/SalmonEgg/SalmonEgg/Styles/ChatStyles.xaml +++ b/SalmonEgg/SalmonEgg/Styles/ChatStyles.xaml @@ -180,6 +180,7 @@ CopyCommandParameter="{x:Bind DisplayBodyText, Mode=OneWay}" ReportCommand="{x:Bind ReportContentCommand}" ToolTitle="{x:Bind Title, Mode=OneWay}" + ToolName="{x:Bind ToolCallName, Mode=OneWay}" ToolKind="{x:Bind ToolCallKind, Mode=OneWay}" Status="{x:Bind ToolCallStatus, Mode=OneWay}" IsInProgress="{x:Bind IsToolCallInProgress, Mode=OneWay}" diff --git a/src/SalmonEgg.Acp/Client/AcpClient.cs b/src/SalmonEgg.Acp/Client/AcpClient.cs index f00f524c..26fabebd 100644 --- a/src/SalmonEgg.Acp/Client/AcpClient.cs +++ b/src/SalmonEgg.Acp/Client/AcpClient.cs @@ -2930,7 +2930,8 @@ private void HandleDraftPermissionRequest(JsonRpcRequest request) { DraftRequest = snapshot, Title = snapshot.Title, - Description = snapshot.Description + Description = snapshot.Description, + ToolCallName = ReadDraftSubjectToolCallName(snapshot) }; pending.PermissionEvent = eventArgs; } @@ -2945,6 +2946,15 @@ private void HandleDraftPermissionRequest(JsonRpcRequest request) PublishPermissionRequest(pending, eventArgs); } + // v2 moved the tool call off the request and onto a subject union, so the permission event + // args cannot extract it the way the v1 path does. Pull the tool_call variant's name here, + // where the draft snapshot is in hand, instead of leaving the label blank on every v2 + // approval - which is exactly the case the name exists to disambiguate. + private static string? ReadDraftSubjectToolCallName(AcpPermissionRequestSnapshot snapshot) + => snapshot.Subject is ToolCallPermissionSubject { ToolCall: { Name: { } name } } + ? name + : null; + private void PublishPermissionRequest(PendingInboundRequest pending, PermissionRequestEventArgs eventArgs) { try diff --git a/src/SalmonEgg.Acp/Client/AcpSessionProjection.cs b/src/SalmonEgg.Acp/Client/AcpSessionProjection.cs index 67e9ca0c..6194ec57 100644 --- a/src/SalmonEgg.Acp/Client/AcpSessionProjection.cs +++ b/src/SalmonEgg.Acp/Client/AcpSessionProjection.cs @@ -143,6 +143,7 @@ private static JsonElement StoreTool(string toolCallId, ToolState state) { writer.WriteStartObject(); writer.WriteString("toolCallId", toolCallId); + writer.WriteString("name", state.Name); writer.WriteString("title", state.Title); writer.WriteString("kind", state.Kind?.Value); writer.WriteString("status", state.Status?.Value); @@ -215,6 +216,7 @@ private void AppendMessage(string messageId, AcpMessageKind kind, ContentBlock? private void ApplyTool(ToolCallStatusUpdate update, JsonElement payload) { var tool = GetTool(update.ToolCallId!); + if (payload.TryGetProperty("name", out _)) tool.Name = update.Name; if (payload.TryGetProperty("title", out _)) tool.Title = update.Title; if (payload.TryGetProperty("kind", out _)) tool.Kind = update.Kind; if (payload.TryGetProperty("status", out _)) tool.Status = update.Status; @@ -334,6 +336,7 @@ internal AcpMessageSnapshot Snapshot() private sealed class ToolState(string id) { + internal string? Name { get; set; } internal string? Title { get; set; } internal ToolCallKind? Kind { get; set; } internal ToolCallStatus? Status { get; set; } @@ -344,7 +347,7 @@ private sealed class ToolState(string id) internal JsonElement? Meta { get; set; } internal Dictionary ExtensionData { get; } = new(StringComparer.Ordinal); internal AcpToolCallSnapshot Snapshot() - => new(id, Title, Kind, Status, Content.ToImmutableArray(), Locations.ToImmutableArray(), RawInput, RawOutput, + => new(id, Name, Title, Kind, Status, Content.ToImmutableArray(), Locations.ToImmutableArray(), RawInput, RawOutput, Meta, ExtensionData.ToImmutableDictionary(StringComparer.Ordinal)); } diff --git a/src/SalmonEgg.Acp/Client/AcpSessionSnapshot.cs b/src/SalmonEgg.Acp/Client/AcpSessionSnapshot.cs index 8b114ec3..a5bef3af 100644 --- a/src/SalmonEgg.Acp/Client/AcpSessionSnapshot.cs +++ b/src/SalmonEgg.Acp/Client/AcpSessionSnapshot.cs @@ -139,6 +139,7 @@ public sealed class AcpToolCallSnapshot internal AcpToolCallSnapshot( string toolCallId, + string? name, string? title, ToolCallKind? kind, ToolCallStatus? status, @@ -150,6 +151,7 @@ internal AcpToolCallSnapshot( ImmutableDictionary extensionData) { ToolCallId = toolCallId; + Name = name; Title = title; Kind = kind; Status = status; @@ -163,6 +165,8 @@ internal AcpToolCallSnapshot( /// The tool call id supplied by the Agent. public string ToolCallId { get; } + /// The latest programmatic tool name, or null when unknown or cleared. + public string? Name { get; } /// The latest title, or null when unknown or cleared. public string? Title { get; } /// The latest kind, preserving unknown protocol values. diff --git a/src/SalmonEgg.Acp/Client/IAcpClient.cs b/src/SalmonEgg.Acp/Client/IAcpClient.cs index df5b22bd..29605ca1 100644 --- a/src/SalmonEgg.Acp/Client/IAcpClient.cs +++ b/src/SalmonEgg.Acp/Client/IAcpClient.cs @@ -363,6 +363,7 @@ public PermissionRequestEventArgs( SessionId = sessionId; ToolCall = toolCall; Title = ReadToolCallTitle(toolCall); + ToolCallName = ReadToolCallName(toolCall); Options = options; Respond = respond; } @@ -422,6 +423,9 @@ internal PermissionRequestEventArgs( /// The permission title supplied by the peer, or null when the protocol omits it. public string? Title { get; init; } + /// The agent's programmatic name for the tool being authorized, or null when unknown. + public string? ToolCallName { get; init; } + /// The optional explanation supplied by the peer. public string? Description { get; init; } @@ -540,6 +544,17 @@ private void LogObserverFailure(Exception error) && title.ValueKind == JsonValueKind.String => title.GetString(), _ => null }; + + // ACP v1 requires the tool call on every permission request, and v2 carries it on the + // subject's tool_call variant, so the programmatic name rides the same payload as the title. + private static string? ReadToolCallName(object? toolCall) + => toolCall switch + { + ToolCallUpdate update => update.Name, + JsonElement { ValueKind: JsonValueKind.Object } value when value.TryGetProperty("name", out var name) + && name.ValueKind == JsonValueKind.String => name.GetString(), + _ => null + }; } public enum FileSystemRequestKind diff --git a/src/SalmonEgg.Acp/Protocol/SessionUpdateTypes.cs b/src/SalmonEgg.Acp/Protocol/SessionUpdateTypes.cs index 470f9b68..f4c7f159 100644 --- a/src/SalmonEgg.Acp/Protocol/SessionUpdateTypes.cs +++ b/src/SalmonEgg.Acp/Protocol/SessionUpdateTypes.cs @@ -744,6 +744,23 @@ public sealed record ToolCallUpdate : SessionUpdate [JsonPropertyName("toolCallId")] public string? ToolCallId { get; init; } + /// + /// Programmatic name of the invoked tool, such as read_file (optional). + /// + /// + /// Stabilized in ACP v1 and v2 by the Tool Call Name RFD. The spelling is opaque: ACP assigns no + /// behavioral or authorization semantics to it, and the name may be reused by many calls, so it + /// identifies the tool rather than the invocation ( does that) and is + /// distinct from the human-readable . + /// + /// Null and absent both mean "no name reported"; neither the v1 nor the v2 update semantics + /// require a client to distinguish them, and nothing in this SDK merges the field into + /// accumulated state, so presence is deliberately not tracked here. + /// + /// + [JsonPropertyName("name")] + public string? Name { get; init; } + /// /// Tool call kind. /// @@ -804,6 +821,11 @@ public ToolCallUpdate() /// List of file locations. /// Raw input parameters. /// Raw output result. + /// + /// is deliberately not a parameter: adding one would change this published + /// constructor's signature, which is binary-breaking for the shipped package even though an + /// optional parameter looks source-compatible. Set it through the init-only property. + /// public ToolCallUpdate( string? toolCallId = null, ToolCallKind? kind = null, @@ -893,6 +915,24 @@ public sealed record ToolCallStatusUpdate : SessionUpdate [JsonPropertyName("toolCallId")] public string? ToolCallId { get; init; } + /// + /// Programmatic name of the invoked tool, such as read_file (optional). + /// + /// + /// Stabilized in ACP v1 and v2 by the Tool Call Name RFD, on the same upsert shape both versions + /// share. The spelling is opaque: ACP assigns no behavioral or authorization semantics to it, and + /// the name may be reused by many calls, so it identifies the tool rather than the invocation + /// ( does that) and is distinct from the human-readable . + /// + /// Null and absent both mean "no name reported". The versions differ on what an explicit null + /// means to a consumer that merges updates — v2 reads it as "clear the name", v1 cannot clear a + /// previously reported one — but neither difference is observable here, because this SDK does not + /// merge the field into accumulated state. Track presence only at such a merge point. + /// + /// + [JsonPropertyName("name")] + public string? Name { get; init; } + /// /// Tool call kind. /// diff --git a/src/SalmonEgg.Acp/Serialization/SessionProjectionWireContract.cs b/src/SalmonEgg.Acp/Serialization/SessionProjectionWireContract.cs index 8ef6b4d4..2eb54068 100644 --- a/src/SalmonEgg.Acp/Serialization/SessionProjectionWireContract.cs +++ b/src/SalmonEgg.Acp/Serialization/SessionProjectionWireContract.cs @@ -36,6 +36,7 @@ internal static void Apply(JsonTypeInfo info) RequireId(info, "toolCallId", static value => value is ToolCallStatusUpdate patch ? patch.ToolCallId : ((ToolCallUpdate)value).ToolCallId); Property(info, "title").CustomConverter = new DefaultableStringJsonConverter(); + Property(info, "name").CustomConverter = new DefaultableStringJsonConverter(); Property(info, "kind").CustomConverter = new DefaultableNullableJsonConverter(); Property(info, "status").CustomConverter = new DefaultableNullableJsonConverter(); Property(info, "content").CustomConverter = new DefaultableListJsonConverter(); diff --git a/src/SalmonEgg.Domain/Models/Conversation/ConversationDocument.cs b/src/SalmonEgg.Domain/Models/Conversation/ConversationDocument.cs index 153a72a1..ad210a60 100644 --- a/src/SalmonEgg.Domain/Models/Conversation/ConversationDocument.cs +++ b/src/SalmonEgg.Domain/Models/Conversation/ConversationDocument.cs @@ -224,6 +224,13 @@ public sealed class ConversationMessageSnapshot public string? ToolCallId { get; set; } + /// + /// Programmatic name of the invoked tool reported by the agent (for example read_file). + /// Distinct from , which is human-readable copy for one invocation. + /// Null when the agent reported none; the field is optional in both ACP versions. + /// + public string? ToolCallName { get; set; } + /// /// Open ACP tool-call kind wire value (for example read, execute). /// diff --git a/src/SalmonEgg.Presentation.Core/Mvux/Chat/ChatReducer.cs b/src/SalmonEgg.Presentation.Core/Mvux/Chat/ChatReducer.cs index 0ed1742e..c166032c 100644 --- a/src/SalmonEgg.Presentation.Core/Mvux/Chat/ChatReducer.cs +++ b/src/SalmonEgg.Presentation.Core/Mvux/Chat/ChatReducer.cs @@ -705,6 +705,7 @@ private static ConversationMessageSnapshot CloneMessage( AudioMimeType = source.AudioMimeType, ProtocolMessageId = source.ProtocolMessageId, ToolCallId = source.ToolCallId, + ToolCallName = source.ToolCallName, ToolCallKind = source.ToolCallKind, ToolCallStatus = source.ToolCallStatus, ToolCallJson = source.ToolCallJson, diff --git a/src/SalmonEgg.Presentation.Core/Services/Chat/ChatConversationWorkspace.cs b/src/SalmonEgg.Presentation.Core/Services/Chat/ChatConversationWorkspace.cs index 0b412e3a..17f59c84 100644 --- a/src/SalmonEgg.Presentation.Core/Services/Chat/ChatConversationWorkspace.cs +++ b/src/SalmonEgg.Presentation.Core/Services/Chat/ChatConversationWorkspace.cs @@ -1329,6 +1329,7 @@ private static ConversationMessageSnapshot CloneMessage(ConversationMessageSnaps AudioMimeType = source.AudioMimeType, ProtocolMessageId = source.ProtocolMessageId, ToolCallId = source.ToolCallId, + ToolCallName = source.ToolCallName, ToolCallKind = source.ToolCallKind, ToolCallStatus = source.ToolCallStatus, ToolCallJson = source.ToolCallJson, diff --git a/src/SalmonEgg.Presentation.Core/Services/Chat/OutgoingUserMessageProjector.cs b/src/SalmonEgg.Presentation.Core/Services/Chat/OutgoingUserMessageProjector.cs index 9d8df88c..d004ebae 100644 --- a/src/SalmonEgg.Presentation.Core/Services/Chat/OutgoingUserMessageProjector.cs +++ b/src/SalmonEgg.Presentation.Core/Services/Chat/OutgoingUserMessageProjector.cs @@ -129,6 +129,7 @@ private static ConversationMessageSnapshot CloneSnapshot(ConversationMessageSnap AudioMimeType = snapshot.AudioMimeType, ProtocolMessageId = snapshot.ProtocolMessageId, ToolCallId = snapshot.ToolCallId, + ToolCallName = snapshot.ToolCallName, ToolCallKind = snapshot.ToolCallKind, ToolCallStatus = snapshot.ToolCallStatus, ToolCallJson = snapshot.ToolCallJson, diff --git a/src/SalmonEgg.Presentation.Core/Services/Chat/WorkspaceWriter.cs b/src/SalmonEgg.Presentation.Core/Services/Chat/WorkspaceWriter.cs index aa8b6b23..e856e94d 100644 --- a/src/SalmonEgg.Presentation.Core/Services/Chat/WorkspaceWriter.cs +++ b/src/SalmonEgg.Presentation.Core/Services/Chat/WorkspaceWriter.cs @@ -645,6 +645,7 @@ private static bool MessageEquals(ConversationMessageSnapshot left, Conversation && string.Equals(left.AudioMimeType, right.AudioMimeType, StringComparison.Ordinal) && string.Equals(left.ProtocolMessageId, right.ProtocolMessageId, StringComparison.Ordinal) && string.Equals(left.ToolCallId, right.ToolCallId, StringComparison.Ordinal) + && string.Equals(left.ToolCallName, right.ToolCallName, StringComparison.Ordinal) && left.ToolCallKind == right.ToolCallKind && left.ToolCallStatus == right.ToolCallStatus && string.Equals(left.ToolCallJson, right.ToolCallJson, StringComparison.Ordinal) @@ -818,6 +819,7 @@ private static ConversationMessageSnapshot CloneMessageSnapshot(ConversationMess AudioMimeType = snapshot.AudioMimeType, ProtocolMessageId = snapshot.ProtocolMessageId, ToolCallId = snapshot.ToolCallId, + ToolCallName = snapshot.ToolCallName, ToolCallKind = snapshot.ToolCallKind, ToolCallStatus = snapshot.ToolCallStatus, ToolCallJson = snapshot.ToolCallJson, diff --git a/src/SalmonEgg.Presentation.Core/ViewModels/Chat/ChatMessageViewModel.cs b/src/SalmonEgg.Presentation.Core/ViewModels/Chat/ChatMessageViewModel.cs index 53cc21d7..e26d840f 100644 --- a/src/SalmonEgg.Presentation.Core/ViewModels/Chat/ChatMessageViewModel.cs +++ b/src/SalmonEgg.Presentation.Core/ViewModels/Chat/ChatMessageViewModel.cs @@ -71,6 +71,13 @@ public partial class ChatMessageViewModel : ObservableObject, IRenderFailureSink [NotifyPropertyChangedFor(nameof(ShouldShowToolCallPill))] private string? _toolCallId; + /// + /// The agent's programmatic name for the invoked tool (for example read_file). Rendered as a + /// secondary label under the title; null when the agent reported none. + /// + [ObservableProperty] + private string? _toolCallName; + [ObservableProperty] [NotifyPropertyChangedFor(nameof(ShouldShowToolCallPill))] private SalmonEgg.Acp.Tool.ToolCallKind? _toolCallKind; @@ -439,6 +446,7 @@ public void ApplySnapshot(ConversationMessageSnapshot snapshot, int projectionIn AudioData = snapshot.AudioData ?? string.Empty; AudioMimeType = snapshot.AudioMimeType ?? string.Empty; ToolCallId = snapshot.ToolCallId; + ToolCallName = snapshot.ToolCallName; ToolCallKind = ToolCallContentSnapshots.ParseKind(snapshot.ToolCallKind); ToolCallStatus = ToolCallContentSnapshots.ParseStatus(snapshot.ToolCallStatus); ToolCallJson = snapshot.ToolCallJson; diff --git a/src/SalmonEgg.Presentation.Core/ViewModels/Chat/ChatViewModel.AcpSessionLifecycle.cs b/src/SalmonEgg.Presentation.Core/ViewModels/Chat/ChatViewModel.AcpSessionLifecycle.cs index b1740dad..74bdb76d 100644 --- a/src/SalmonEgg.Presentation.Core/ViewModels/Chat/ChatViewModel.AcpSessionLifecycle.cs +++ b/src/SalmonEgg.Presentation.Core/ViewModels/Chat/ChatViewModel.AcpSessionLifecycle.cs @@ -2130,6 +2130,7 @@ private PermissionRequestViewModel CreateOwnedPermissionRequest( }, () => PostToUiAsync(() => RemovePermissionRequestProjection(conversationId, viewModel!))); viewModel.ToolCallId = TryResolvePermissionToolCallId(request.ToolCall); + viewModel.ToolCallName = request.ToolCallName; viewModel.RequestTitle = request.Title; viewModel.Title = string.IsNullOrWhiteSpace(viewModel.RequestTitle) ? ResolveLocalizerText("Permission_DefaultTitle", "Permission required") : viewModel.RequestTitle; @@ -2685,6 +2686,7 @@ private ConversationMessageSnapshot CreateToolCallSnapshot(ToolCallUpdate toolCa Title = toolCall.Title ?? string.Empty, TextContent = ResolveToolCallOutput(toolCall.RawOutput, toolCall.Content, string.Empty), ToolCallId = toolCall.ToolCallId, + ToolCallName = toolCall.Name, ToolCallKind = ToolCallContentSnapshots.FormatKind(toolCall.Kind), ToolCallStatus = ToolCallContentSnapshots.FormatStatus(toolCall.Status), ToolCallJson = ResolveToolCallPayload(toolCall.RawInput, toolCall.Content), @@ -2739,6 +2741,10 @@ private async Task UpdateToolCallStatusAsync(string? conversationId, ToolCallSta AudioMimeType = existing.AudioMimeType, ProtocolMessageId = existing.ProtocolMessageId, ToolCallId = existing.ToolCallId, + // v1 cannot clear a reported name, so an absent-or-null patch keeps what the first + // report established. v2 reads an explicit null as a clear; that divergence becomes + // observable only once a v2 connection is served. + ToolCallName = toolCallStatusUpdate.Name ?? existing.ToolCallName, ToolCallKind = toolCallStatusUpdate.Kind is null ? existing.ToolCallKind : ToolCallContentSnapshots.FormatKind(toolCallStatusUpdate.Kind), @@ -2773,6 +2779,7 @@ private ConversationMessageSnapshot CreateToolCallSnapshot(ToolCallStatusUpdate Title = toolCallStatusUpdate.Title ?? string.Empty, TextContent = ResolveToolCallOutput(toolCallStatusUpdate.RawOutput, toolCallStatusUpdate.Content, string.Empty), ToolCallId = toolCallStatusUpdate.ToolCallId, + ToolCallName = toolCallStatusUpdate.Name, ToolCallKind = ToolCallContentSnapshots.FormatKind(toolCallStatusUpdate.Kind), ToolCallStatus = ToolCallContentSnapshots.FormatStatus(toolCallStatusUpdate.Status), ToolCallJson = ResolveToolCallPayload(toolCallStatusUpdate.RawInput, toolCallStatusUpdate.Content), @@ -2930,6 +2937,7 @@ private async Task PreemptivelyCancelOutstandingToolCallsAsync(ChatState state, AudioMimeType = existing.AudioMimeType, ProtocolMessageId = existing.ProtocolMessageId, ToolCallId = existing.ToolCallId, + ToolCallName = existing.ToolCallName, ToolCallKind = existing.ToolCallKind, ToolCallStatus = SalmonEgg.Acp.Tool.ToolCallStatus.Cancelled.ToString(), ToolCallJson = existing.ToolCallJson, diff --git a/src/SalmonEgg.Presentation.Core/ViewModels/Chat/ChatViewModel.cs b/src/SalmonEgg.Presentation.Core/ViewModels/Chat/ChatViewModel.cs index 366c203f..a6787713 100644 --- a/src/SalmonEgg.Presentation.Core/ViewModels/Chat/ChatViewModel.cs +++ b/src/SalmonEgg.Presentation.Core/ViewModels/Chat/ChatViewModel.cs @@ -2653,6 +2653,7 @@ private static bool MatchesSnapshot(ChatMessageViewModel viewModel, Conversation && string.Equals(viewModel.AudioData ?? string.Empty, snapshot.AudioData ?? string.Empty, StringComparison.Ordinal) && string.Equals(viewModel.AudioMimeType ?? string.Empty, snapshot.AudioMimeType ?? string.Empty, StringComparison.Ordinal) && string.Equals(viewModel.ToolCallId, snapshot.ToolCallId, StringComparison.Ordinal) + && string.Equals(viewModel.ToolCallName, snapshot.ToolCallName, StringComparison.Ordinal) && viewModel.ToolCallKind == ToolCallContentSnapshots.ParseKind(snapshot.ToolCallKind) && viewModel.ToolCallStatus == ToolCallContentSnapshots.ParseStatus(snapshot.ToolCallStatus) && string.Equals(viewModel.ToolCallJson, snapshot.ToolCallJson, StringComparison.Ordinal) @@ -2697,6 +2698,7 @@ private static ConversationMessageSnapshot CloneSnapshot(ConversationMessageSnap AudioMimeType = snapshot.AudioMimeType, ProtocolMessageId = snapshot.ProtocolMessageId, ToolCallId = snapshot.ToolCallId, + ToolCallName = snapshot.ToolCallName, ToolCallKind = snapshot.ToolCallKind, ToolCallStatus = snapshot.ToolCallStatus, ToolCallJson = snapshot.ToolCallJson, diff --git a/src/SalmonEgg.Presentation.Core/ViewModels/Chat/PermissionRequestViewModel.cs b/src/SalmonEgg.Presentation.Core/ViewModels/Chat/PermissionRequestViewModel.cs index 51e3c47b..94ed4ec2 100644 --- a/src/SalmonEgg.Presentation.Core/ViewModels/Chat/PermissionRequestViewModel.cs +++ b/src/SalmonEgg.Presentation.Core/ViewModels/Chat/PermissionRequestViewModel.cs @@ -25,6 +25,16 @@ public partial class PermissionRequestViewModel : ObservableObject public string ToolCallJson { get; set; } = string.Empty; internal string? ToolCallId { get; set; } internal string? RequestTitle { get; set; } + + // Observable, not a plain property: the pill binds both this and HasToolCallName, and the + // cancellation-retry path clears the name on an already-projected instance. Without a change + // notification the InfoBar would keep showing a tool the card no longer offers. + [ObservableProperty] + [NotifyPropertyChangedFor(nameof(HasToolCallName))] + private string? _toolCallName; + + public bool HasToolCallName => !string.IsNullOrWhiteSpace(ToolCallName); + internal ConversationBindingSlice? Binding { get; set; } internal Task? BindingCancellationTask { get; set; } internal bool BindingCancellationAttempted { get; set; } @@ -45,6 +55,7 @@ internal void ShowCancellationRetry(string title, string description, bool bindi { if (bindingChanged) BindingCancellationAttempted = true; ToolCallId = null; + ToolCallName = null; Title = title; Description = description; if (_showsCancellationRetry) diff --git a/src/SalmonEgg.Presentation.Core/ViewModels/Chat/Transcript/ChatTranscriptVirtualizedMessageCollection.cs b/src/SalmonEgg.Presentation.Core/ViewModels/Chat/Transcript/ChatTranscriptVirtualizedMessageCollection.cs index 3e9d7dc5..5d6a15dc 100644 --- a/src/SalmonEgg.Presentation.Core/ViewModels/Chat/Transcript/ChatTranscriptVirtualizedMessageCollection.cs +++ b/src/SalmonEgg.Presentation.Core/ViewModels/Chat/Transcript/ChatTranscriptVirtualizedMessageCollection.cs @@ -434,6 +434,7 @@ private static bool SnapshotProjectionEquals( && string.Equals(oldSnapshot.AudioMimeType ?? string.Empty, newSnapshot.AudioMimeType ?? string.Empty, StringComparison.Ordinal) && string.Equals(oldSnapshot.ProtocolMessageId, newSnapshot.ProtocolMessageId, StringComparison.Ordinal) && string.Equals(oldSnapshot.ToolCallId, newSnapshot.ToolCallId, StringComparison.Ordinal) + && string.Equals(oldSnapshot.ToolCallName, newSnapshot.ToolCallName, StringComparison.Ordinal) && oldSnapshot.ToolCallKind == newSnapshot.ToolCallKind && oldSnapshot.ToolCallStatus == newSnapshot.ToolCallStatus && string.Equals(oldSnapshot.ToolCallJson, newSnapshot.ToolCallJson, StringComparison.Ordinal) diff --git a/tests/SalmonEgg.Acp.Tests/Client/AcpDraftPermissionTests.cs b/tests/SalmonEgg.Acp.Tests/Client/AcpDraftPermissionTests.cs index ffef7776..43170240 100644 --- a/tests/SalmonEgg.Acp.Tests/Client/AcpDraftPermissionTests.cs +++ b/tests/SalmonEgg.Acp.Tests/Client/AcpDraftPermissionTests.cs @@ -89,6 +89,41 @@ public async Task PermissionRequest_CommandSubject_ExposesContextWithoutCreating Assert.Single(peer.Responses); } + // v2 moved the tool call off the permission request onto a subject union, so the v1 extraction + // never sees it. Without this the approval label is blank on every v2 request - the one case where + // naming the tool matters most, because the title is the only other thing on screen. + [Fact] + public async Task PermissionRequest_ToolSubject_CarriesTheProgrammaticToolName() + { + // Arrange + using var peer = await PermissionPeer.CreateAsync(); + + // Act + peer.Request("{\"sessionId\":\"one\",\"title\":\"Run tests?\"," + + "\"subject\":{\"type\":\"tool_call\",\"toolCall\":{\"toolCallId\":\"tool\",\"name\":\"run_command\",\"title\":\"subject title\"}}," + + Options + "}"); + var request = Assert.Single(peer.Requests); + + // Assert + Assert.Equal("run_command", request.ToolCallName); + } + + [Fact] + public async Task PermissionRequest_WithoutAToolSubject_LeavesTheToolNameUnset() + { + // Arrange + using var peer = await PermissionPeer.CreateAsync(); + + // Act + peer.Request("{\"sessionId\":\"one\",\"title\":\"Run tests?\"," + + "\"subject\":{\"type\":\"command\",\"command\":\"cargo test\",\"cwd\":\"/work\"}," + + Options + "}"); + var request = Assert.Single(peer.Requests); + + // Assert + Assert.Null(request.ToolCallName); + } + [Fact] public async Task PermissionRequest_ToolSubject_DoesNotOverwriteToolProjectionOrPromptText() { diff --git a/tests/SalmonEgg.Acp.Tests/Protocol/ToolCallNameTests.cs b/tests/SalmonEgg.Acp.Tests/Protocol/ToolCallNameTests.cs new file mode 100644 index 00000000..789a5574 --- /dev/null +++ b/tests/SalmonEgg.Acp.Tests/Protocol/ToolCallNameTests.cs @@ -0,0 +1,96 @@ +using System.Text.Json; +using SalmonEgg.Acp.Protocol; +using SalmonEgg.Acp.Serialization; +using Xunit; + +namespace SalmonEgg.Acp.Tests.Protocol; + +/// +/// The Tool Call Name RFD stabilized an optional name on tool calls in ACP v1 and v2. This SDK +/// already carried it losslessly inside as an unknown field; +/// these cases pin the part that was missing — a typed, readable, round-trippable name on both the v1 +/// tool_call shape and the upsert shape the two versions share. +/// +public sealed class ToolCallNameTests +{ + private static SessionUpdate? Parse(int version, string updateJson) => + JsonSerializer.Deserialize( + $"{{\"sessionId\":\"session-1\",\"update\":{updateJson}}}", + AcpWireFormat.For(version).TypeInfo())?.Update; + + private static string RoundTrip(int version, string updateJson) + { + var update = Parse(version, updateJson); + return JsonSerializer.Serialize( + new SessionUpdateParams { SessionId = "session-1", Update = update! }, + AcpWireFormat.For(version).TypeInfo()); + } + + [Fact] + public void V1ToolCall_ExposesTheProgrammaticName() + { + var update = Assert.IsType(Parse( + AcpProtocolVersion.V1, + "{\"sessionUpdate\":\"tool_call\",\"toolCallId\":\"tc-1\",\"name\":\"read_file\"," + + "\"title\":\"Reading configuration file\",\"kind\":\"read\"}")); + + Assert.Equal("read_file", update.Name); + // A name that stayed in ExtensionData would read as "absent" here while still round-tripping, + // so the assertion above alone cannot tell the two apart; this one can. + Assert.False(update.ExtensionData?.ContainsKey("name") == true); + } + + [Theory] + [InlineData(AcpProtocolVersion.V1)] + [InlineData(AcpProtocolVersion.V2)] + public void ToolCallUpdate_ExposesTheProgrammaticName(int version) + { + var update = Assert.IsType(Parse( + version, + "{\"sessionUpdate\":\"tool_call_update\",\"toolCallId\":\"tc-1\",\"name\":\"run_command\"," + + "\"status\":\"in_progress\"}")); + + Assert.Equal("run_command", update.Name); + Assert.False(update.ExtensionData?.ContainsKey("name") == true); + } + + // The name is informational, so a proxy that only forwards updates it cannot interpret still has to + // reproduce it — that is the one capability loss the SDK itself has to rule out. + [Theory] + [InlineData(AcpProtocolVersion.V1, "tool_call")] + [InlineData(AcpProtocolVersion.V1, "tool_call_update")] + [InlineData(AcpProtocolVersion.V2, "tool_call_update")] + public void ToolCallName_SurvivesAFullRoundTrip(int version, string discriminator) + { + using var replayed = JsonDocument.Parse( + RoundTrip(version, $"{{\"sessionUpdate\":\"{discriminator}\",\"toolCallId\":\"tc-1\",\"name\":\"grep\"}}")); + + Assert.Equal("grep", replayed.RootElement.GetProperty("update").GetProperty("name").GetString()); + } + + // v2 grants default-on-error to its optional patch fields, and every sibling on these shapes + // carries the converter. Without one for name, a peer sending a non-string would drop the whole + // update - status, title and all - where the identical payload with a bad "title" recovers to + // null and renders fine. Scoped to v2 because that is the only surface the recovery contract + // applies to; v1 has no such grant and stays strict on purpose. + [Theory] + [InlineData("tool_call_update")] + public void V2NonStringName_DefaultsInsteadOfDroppingTheWholeUpdate(string discriminator) + { + var update = Parse(AcpProtocolVersion.V2, + $"{{\"sessionUpdate\":\"{discriminator}\",\"toolCallId\":\"tc-1\",\"name\":123,\"status\":\"completed\"}}"); + + Assert.IsType(update); + Assert.Null(((ToolCallStatusUpdate)update!).Name); + } + + [Fact] + public void ToolCallUpdate_WithoutAName_ReadsAsAbsentRatherThanEmpty() + { + var update = Assert.IsType(Parse( + AcpProtocolVersion.V1, + "{\"sessionUpdate\":\"tool_call_update\",\"toolCallId\":\"tc-1\",\"status\":\"completed\"}")); + + Assert.Null(update.Name); + } +} diff --git a/tests/SalmonEgg.Presentation.Core.Tests/Chat/ChatMessageViewModelToolCallNameTests.cs b/tests/SalmonEgg.Presentation.Core.Tests/Chat/ChatMessageViewModelToolCallNameTests.cs new file mode 100644 index 00000000..6c4bf818 --- /dev/null +++ b/tests/SalmonEgg.Presentation.Core.Tests/Chat/ChatMessageViewModelToolCallNameTests.cs @@ -0,0 +1,49 @@ +using SalmonEgg.Domain.Models.Conversation; +using SalmonEgg.Presentation.ViewModels.Chat; + +namespace SalmonEgg.Presentation.Core.Tests.Chat; + +/// +/// The agent's programmatic tool name (ACP tool-call name) has to survive the whole projection +/// chain — protocol snapshot, then display view model — or the label the pill renders never lights up. +/// +public sealed class ChatMessageViewModelToolCallNameTests +{ + [Fact] + public void ReportedToolName_ReachesTheDisplayViewModel() + { + var vm = new ChatMessageViewModel(); + + vm.ApplySnapshot( + new ConversationMessageSnapshot + { + Id = "tool-name", + ContentType = "tool_call", + Title = "Reading config", + ToolCallId = "call-1", + ToolCallName = "read_file" + }, + projectionIndex: 0); + + Assert.Equal("read_file", vm.ToolCallName); + Assert.True(vm.ShouldShowToolCallPill); + } + + [Fact] + public void UnreportedToolName_StaysUnset() + { + var vm = new ChatMessageViewModel(); + + vm.ApplySnapshot( + new ConversationMessageSnapshot + { + Id = "tool-no-name", + ContentType = "tool_call", + Title = "Reading config", + ToolCallId = "call-2" + }, + projectionIndex: 0); + + Assert.Null(vm.ToolCallName); + } +} \ No newline at end of file diff --git a/tests/SalmonEgg.Presentation.Core.Tests/Chat/ChatViewModelTests.PermissionOwnership.cs b/tests/SalmonEgg.Presentation.Core.Tests/Chat/ChatViewModelTests.PermissionOwnership.cs index 939cfdb3..22be6ba5 100644 --- a/tests/SalmonEgg.Presentation.Core.Tests/Chat/ChatViewModelTests.PermissionOwnership.cs +++ b/tests/SalmonEgg.Presentation.Core.Tests/Chat/ChatViewModelTests.PermissionOwnership.cs @@ -46,6 +46,40 @@ public async Task PermissionRequest_OldQueuedEventAfterConnectionReplacement_Doe Assert.Empty(currentPeer.Responses); } + [Fact] + public async Task PermissionRequest_CarriesTheAgentsProgrammaticToolName() + { + var dispatcher = new QueueingSynchronizationContext(); + await using var fixture = CreateInteractionViewModel(dispatcher); + using var peer = await PermissionUiPeer.CreateAsync(); + await AttachPermissionPeerAsync(fixture, dispatcher, peer); + + peer.Request("permission", "remote-1", "tool-1", toolName: "run_command"); + await dispatcher.RunUntilIdleAsync(); + + // Approving "Run tests" without knowing it is run_command is the gap the RFD closes: the + // title is human copy and two different tools can produce it. + var request = Assert.IsType(fixture.ViewModel.PendingPermissionRequest); + Assert.Equal("run_command", request.ToolCallName); + Assert.True(request.HasToolCallName); + } + + [Fact] + public async Task PermissionRequest_WithoutAToolName_LeavesTheLabelUnset() + { + var dispatcher = new QueueingSynchronizationContext(); + await using var fixture = CreateInteractionViewModel(dispatcher); + using var peer = await PermissionUiPeer.CreateAsync(); + await AttachPermissionPeerAsync(fixture, dispatcher, peer); + + peer.Request("permission", "remote-1", "tool-1"); + await dispatcher.RunUntilIdleAsync(); + + var request = Assert.IsType(fixture.ViewModel.PendingPermissionRequest); + Assert.Null(request.ToolCallName); + Assert.False(request.HasToolCallName); + } + [Fact] public async Task PermissionRequest_OldCommandAfterConnectionReplacement_CannotAnswerReusedId() { @@ -747,8 +781,15 @@ public async Task ReconnectAsync() public Task DisconnectClientAsync() => _client.DisconnectAsync(); - public void Request(string id, string sessionId, string toolCallId) - => Receive($$$"""{"jsonrpc":"2.0","id":"{{{id}}}","method":"session/request_permission","params":{"sessionId":"{{{sessionId}}}","toolCall":{"toolCallId":"{{{toolCallId}}}","title":"Run tests"},"options":[{"optionId":"allow","name":"Allow once","kind":"allow_once"}]}}"""); + public void Request(string id, string sessionId, string toolCallId, string? toolName = null) + { + // ACP carries the programmatic tool name on the same ToolCallUpdate the permission request + // already carries, so the only difference between a named and an unnamed request is this. + var nameJson = toolName is null ? string.Empty : "\"name\":\"" + toolName + "\","; + Receive("{\"jsonrpc\":\"2.0\",\"id\":\"" + id + "\",\"method\":\"session/request_permission\",\"params\":{" + + "\"sessionId\":\"" + sessionId + "\",\"toolCall\":{\"toolCallId\":\"" + toolCallId + "\"," + nameJson + + "\"title\":\"Run tests\"},\"options\":[{\"optionId\":\"allow\",\"name\":\"Allow once\",\"kind\":\"allow_once\"}]}}"); + } public void CancelPermission(string id) => Receive($$$"""{"jsonrpc":"2.0","method":"$/cancel_request","params":{"requestId":"{{{id}}}"}}"""); diff --git a/tests/SalmonEgg.Presentation.Core.Tests/Chat/ChatViewModelTests.VersionedUpdates.cs b/tests/SalmonEgg.Presentation.Core.Tests/Chat/ChatViewModelTests.VersionedUpdates.cs index abd58255..e6244539 100644 --- a/tests/SalmonEgg.Presentation.Core.Tests/Chat/ChatViewModelTests.VersionedUpdates.cs +++ b/tests/SalmonEgg.Presentation.Core.Tests/Chat/ChatViewModelTests.VersionedUpdates.cs @@ -363,4 +363,42 @@ public Task SendMessageAsync(string message, CancellationToken cancellatio } public void Dispose() => Service.Dispose(); } + + [Fact] + public async Task VersionedUpdates_ToolCallName_ReachesTheTranscriptAndSurvivesLaterPatches() + { + var dispatcher = new QueueingSynchronizationContext(); + await using var fixture = CreateInteractionViewModel(dispatcher); + using var stablePeer = await PermissionUiPeer.CreateAsync(); + using var peer = await VersionedUpdatePeer.CreateAsync(); + await AttachPermissionPeerAsync(fixture, dispatcher, stablePeer, peer.Service); + + peer.Update("""{"sessionUpdate":"tool_call_update","toolCallId":"t","name":"read_file","title":"Reading config","status":"in_progress"}"""); + await DrainVersionedUpdatesAsync(fixture, dispatcher); + + var tool = Assert.Single((await fixture.ChatStore.GetCurrentStateAsync()).ResolveContentSlice("conv-1")!.Value.Transcript); + Assert.Equal("read_file", tool.ToolCallName); + Assert.Equal("Reading config", tool.Title); + + // A patch that omits the name means "no change" in both v1 and v2, so the label must survive + // the stream of later updates instead of blinking out on every status change. + peer.Update("""{"sessionUpdate":"tool_call_update","toolCallId":"t","status":"completed"}"""); + await DrainVersionedUpdatesAsync(fixture, dispatcher); + Assert.Equal("read_file", Assert.Single((await fixture.ChatStore.GetCurrentStateAsync()).ResolveContentSlice("conv-1")!.Value.Transcript).ToolCallName); + } + + [Fact] + public async Task VersionedUpdates_ToolCallWithoutAName_LeavesTheLabelUnset() + { + var dispatcher = new QueueingSynchronizationContext(); + await using var fixture = CreateInteractionViewModel(dispatcher); + using var stablePeer = await PermissionUiPeer.CreateAsync(); + using var peer = await VersionedUpdatePeer.CreateAsync(); + await AttachPermissionPeerAsync(fixture, dispatcher, stablePeer, peer.Service); + + peer.Update("""{"sessionUpdate":"tool_call_update","toolCallId":"t","title":"Reading config","status":"completed"}"""); + await DrainVersionedUpdatesAsync(fixture, dispatcher); + + Assert.Null(Assert.Single((await fixture.ChatStore.GetCurrentStateAsync()).ResolveContentSlice("conv-1")!.Value.Transcript).ToolCallName); + } }