diff --git a/agents/Aevatar.GAgents.Channel.Abstractions/protos/chat_activity.proto b/agents/Aevatar.GAgents.Channel.Abstractions/protos/chat_activity.proto index 5f5de960b..6840b1478 100644 --- a/agents/Aevatar.GAgents.Channel.Abstractions/protos/chat_activity.proto +++ b/agents/Aevatar.GAgents.Channel.Abstractions/protos/chat_activity.proto @@ -287,6 +287,16 @@ message TransportExtras { string nyx_user_access_token = 5; // The platform-native message id of the originating user message when Nyx relay can recover it. string nyx_platform_message_id = 6; + // The Lark `union_id` (`on_*`) of the inbound sender. Tenant-stable and cross-app safe, so + // outbound delivery can target the user via `receive_id_type=union_id` even when the inbound + // event was relayed through a different Lark app than the outbound sender. Empty when the + // platform is not Lark or the relay payload could not surface a `union_id`. + string nyx_lark_union_id = 7; + // The Lark `chat_id` (`oc_*`) of the inbound conversation as observed by the relay-side Lark + // app. Cross-app safe within the tenant for groups/threads/channels (any app added to the + // chat can address it via `receive_id_type=chat_id`). For p2p DMs the chat_id is bot-specific + // and not cross-app safe; downstream senders prefer `nyx_lark_union_id` for p2p targets. + string nyx_lark_chat_id = 8; } // Represents one normalized activity flowing through the channel pipeline. diff --git a/agents/Aevatar.GAgents.ChannelRuntime/AgentBuilderCardFlow.cs b/agents/Aevatar.GAgents.ChannelRuntime/AgentBuilderCardFlow.cs index 4acf70fd2..2230847da 100644 --- a/agents/Aevatar.GAgents.ChannelRuntime/AgentBuilderCardFlow.cs +++ b/agents/Aevatar.GAgents.ChannelRuntime/AgentBuilderCardFlow.cs @@ -181,6 +181,13 @@ private static bool TryResolve( decision = AgentBuilderFlowDecision.ToolCall(ListAgentsAction, """{"action":"list_agents"}"""); return true; + case ListTemplatesAction: + // The /agents card surfaces a `Templates` button (also reachable via the + // text-flow `/templates` slash command). Without this branch, clicking the + // button leaves the user with an unhandled card action and no feedback. + decision = AgentBuilderFlowDecision.ToolCall(ListTemplatesAction, """{"action":"list_templates"}"""); + return true; + case AgentStatusAction: if (!TryBuildAgentActionArguments(evt, "agent_status", out argumentsJson, out validationError)) { diff --git a/agents/Aevatar.GAgents.ChannelRuntime/AgentBuilderTool.cs b/agents/Aevatar.GAgents.ChannelRuntime/AgentBuilderTool.cs index f99740634..e5372af32 100644 --- a/agents/Aevatar.GAgents.ChannelRuntime/AgentBuilderTool.cs +++ b/agents/Aevatar.GAgents.ChannelRuntime/AgentBuilderTool.cs @@ -236,10 +236,7 @@ private async Task CreateDailyReportAgentAsync( ?? await actorRuntime.CreateAsync(agentId, ct); var versionBefore = await queryPort.GetStateVersionAsync(agentId, ct) ?? -1; - var deliveryTarget = LarkConversationTargets.BuildFromInbound( - AgentToolRequestContext.TryGet(ChannelMetadataKeys.ChatType), - conversationId, - AgentToolRequestContext.TryGet(ChannelMetadataKeys.SenderId)); + var deliveryTarget = ResolveDeliveryTarget(conversationId, agentId); var initialize = new InitializeSkillRunnerCommand { SkillName = templateSpec.SkillName, @@ -383,10 +380,7 @@ private async Task CreateSocialMediaAgentAsync( ?? await actorRuntime.CreateAsync(agentId, ct); var versionBefore = await queryPort.GetStateVersionAsync(agentId, ct) ?? -1; - var deliveryTarget = LarkConversationTargets.BuildFromInbound( - AgentToolRequestContext.TryGet(ChannelMetadataKeys.ChatType), - conversationId, - AgentToolRequestContext.TryGet(ChannelMetadataKeys.SenderId)); + var deliveryTarget = ResolveDeliveryTarget(conversationId, agentId); var initialize = new InitializeWorkflowAgentCommand { WorkflowId = workflowUpsert.Workflow.WorkflowId, @@ -1417,6 +1411,46 @@ private static string NormalizeScopeId(string? value) => return normalized.Length == 0 ? null : normalized; } + /// + /// Builds the typed Lark delivery target from the current AgentToolRequestContext and emits + /// a LogDebug breadcrumb when falls + /// back from the cross-app safe pair (union_id / chat_id) to the legacy open_id / + /// conversation_id path. The fallback flag is intentionally NOT persisted on + /// SkillRunnerOutboundConfig / InitializeWorkflowAgentCommand because the + /// downstream path treats any populated typed + /// pair as authoritative — so this is the only place the cross-app risk surfaces. Operators + /// correlating Lark code:99992361 open_id cross app rejections need this log line to + /// confirm whether the relay surfaced union_id at agent-create time. + /// + private LarkReceiveTarget ResolveDeliveryTarget(string conversationId, string agentId) + { + var chatType = AgentToolRequestContext.TryGet(ChannelMetadataKeys.ChatType); + var senderId = AgentToolRequestContext.TryGet(ChannelMetadataKeys.SenderId); + var unionId = AgentToolRequestContext.TryGet(ChannelMetadataKeys.LarkUnionId); + var chatId = AgentToolRequestContext.TryGet(ChannelMetadataKeys.LarkChatId); + + var target = LarkConversationTargets.BuildFromInbound( + chatType, + conversationId, + senderId, + unionId, + chatId); + + if (target.FellBackToPrefixInference) + { + _logger?.LogDebug( + "Agent builder fell back to legacy delivery target inference for {AgentId}: chatType={ChatType}, hasUnionId={HasUnionId}, hasLarkChatId={HasLarkChatId}, hasSenderId={HasSenderId}, resolvedReceiveIdType={ReceiveIdType}. Cross-app outbound (e.g. customer api-lark-bot) may surface Lark `99992361 open_id cross app` until the relay propagates union_id.", + agentId, + chatType ?? string.Empty, + !string.IsNullOrWhiteSpace(unionId), + !string.IsNullOrWhiteSpace(chatId), + !string.IsNullOrWhiteSpace(senderId), + target.ReceiveIdType); + } + + return target; + } + private sealed class BuilderArgs { private readonly Dictionary _properties; diff --git a/agents/Aevatar.GAgents.ChannelRuntime/ChannelConversationTurnRunner.cs b/agents/Aevatar.GAgents.ChannelRuntime/ChannelConversationTurnRunner.cs index 5790f9ac9..d55996b75 100644 --- a/agents/Aevatar.GAgents.ChannelRuntime/ChannelConversationTurnRunner.cs +++ b/agents/Aevatar.GAgents.ChannelRuntime/ChannelConversationTurnRunner.cs @@ -681,6 +681,18 @@ private static IReadOnlyDictionary BuildReplyMetadata( if (!string.IsNullOrWhiteSpace(platformMessageId)) metadata[ChannelMetadataKeys.PlatformMessageId] = platformMessageId; + // Lark cross-app outbound delivery: agent-builder consumers prefer the tenant-stable + // union_id / chat_id captured at ingress over the relay-app-scoped open_id, so a + // mismatch between the relay-side Lark app and the customer's outbound Lark app does + // not surface as `code:99992361 open_id cross app` rejections at send time. + var larkUnionId = NormalizeOptional(activity?.TransportExtras?.NyxLarkUnionId); + if (!string.IsNullOrWhiteSpace(larkUnionId)) + metadata[ChannelMetadataKeys.LarkUnionId] = larkUnionId; + + var larkChatId = NormalizeOptional(activity?.TransportExtras?.NyxLarkChatId); + if (!string.IsNullOrWhiteSpace(larkChatId)) + metadata[ChannelMetadataKeys.LarkChatId] = larkChatId; + return metadata; } diff --git a/agents/Aevatar.GAgents.ChannelRuntime/ChannelMetadataKeys.cs b/agents/Aevatar.GAgents.ChannelRuntime/ChannelMetadataKeys.cs index 2eb2b79fe..0ea1fac3b 100644 --- a/agents/Aevatar.GAgents.ChannelRuntime/ChannelMetadataKeys.cs +++ b/agents/Aevatar.GAgents.ChannelRuntime/ChannelMetadataKeys.cs @@ -13,4 +13,19 @@ public static class ChannelMetadataKeys public const string MessageId = "channel.message_id"; public const string PlatformMessageId = "channel.platform_message_id"; public const string ChatType = "channel.chat_type"; + /// + /// Lark union_id (on_*) of the inbound sender. Tenant-stable and cross-app safe; + /// downstream Lark senders prefer this over (open_id) for p2p + /// outbound delivery so a relay-app vs outbound-app mismatch does not produce + /// open_id cross app rejections from Lark. Empty when the platform is not Lark or the + /// relay did not surface a union_id. + /// + public const string LarkUnionId = "channel.lark.union_id"; + /// + /// Lark chat_id (oc_*) as observed by the relay-side Lark app. Cross-app safe + /// within the tenant for groups/threads/channels. Downstream Lark senders prefer this for + /// non-p2p outbound delivery instead of inferring a chat_id from the routing + /// (which may be a NyxID-internal route id). + /// + public const string LarkChatId = "channel.lark.chat_id"; } diff --git a/agents/Aevatar.GAgents.ChannelRuntime/FeishuCardHumanInteractionPort.cs b/agents/Aevatar.GAgents.ChannelRuntime/FeishuCardHumanInteractionPort.cs index 8e085ac4b..289a67633 100644 --- a/agents/Aevatar.GAgents.ChannelRuntime/FeishuCardHumanInteractionPort.cs +++ b/agents/Aevatar.GAgents.ChannelRuntime/FeishuCardHumanInteractionPort.cs @@ -359,13 +359,30 @@ private async Task SendMessageAsync( if (LarkProxyResponse.TryGetError(result, out var larkCode, out var detail)) { - throw new InvalidOperationException( - larkCode is { } code - ? $"{failurePrefix} (code={code}): {detail}" - : $"{failurePrefix}: {detail}"); + throw new InvalidOperationException(BuildLarkRejectionMessage(failurePrefix, larkCode, detail)); } } + private static string BuildLarkRejectionMessage(string failurePrefix, int? larkCode, string detail) + { + if (larkCode == LarkBotErrorCodes.OpenIdCrossApp) + { + // Mirrors the SkillRunnerGAgent recovery hint: the workflow agent's catalog target + // was captured before union_id ingress existed and the persisted typed pair is + // permanently relay-app-scoped. Surface the recreate-the-agent instruction inside + // the exception message so it ends up in `/agent-status`'s `last_error` field + // instead of the cryptic Lark `99992361 open_id cross app`. + return + $"{failurePrefix} (code={larkCode}): {detail}. " + + "This workflow agent was created before cross-app union_id ingress existed; " + + "delete and recreate it (`/agents` → Delete → `/social-media`) to pick up the cross-app safe target."; + } + + return larkCode is { } code + ? $"{failurePrefix} (code={code}): {detail}" + : $"{failurePrefix}: {detail}"; + } + private static bool SupportsApproveReject(HumanInteractionRequest request) => string.Equals(request.SuspensionType, "human_approval", StringComparison.OrdinalIgnoreCase) || (request.Options.Contains("approve", StringComparer.OrdinalIgnoreCase) && diff --git a/agents/Aevatar.GAgents.ChannelRuntime/LarkBotErrorCodes.cs b/agents/Aevatar.GAgents.ChannelRuntime/LarkBotErrorCodes.cs index a717747a2..a546384f2 100644 --- a/agents/Aevatar.GAgents.ChannelRuntime/LarkBotErrorCodes.cs +++ b/agents/Aevatar.GAgents.ChannelRuntime/LarkBotErrorCodes.cs @@ -12,4 +12,15 @@ internal static class LarkBotErrorCodes /// config gap when the bot's app scope is missing the reaction permission. /// public const int NoPermissionToReact = 231002; + + /// + /// "open_id cross app" — Lark open_id is app-scoped (each Lark app issues its own + /// ou_* for the same user). When relay-side ingress (e.g. NyxID's Lark app) and + /// outbound (e.g. customer's api-lark-bot) are different apps, sending to a + /// receive_id_type=open_id with the relay-app-scoped ou_* is rejected. Surfaces + /// on legacy SkillRunner / human-interaction state captured before union_id ingress + /// existed; rebuild the agent (e.g. /agents → Delete → /daily) to pin the new + /// cross-app safe pair. + /// + public const int OpenIdCrossApp = 99992361; } diff --git a/agents/Aevatar.GAgents.ChannelRuntime/LarkConversationTargets.cs b/agents/Aevatar.GAgents.ChannelRuntime/LarkConversationTargets.cs index a56fe48a1..ded90680b 100644 --- a/agents/Aevatar.GAgents.ChannelRuntime/LarkConversationTargets.cs +++ b/agents/Aevatar.GAgents.ChannelRuntime/LarkConversationTargets.cs @@ -57,29 +57,68 @@ public static string ResolveReceiveIdType(string? conversationId) } /// - /// Builds the typed receive-target for a Lark inbound captured at agent creation. For p2p we - /// store the user's open_id (always ou_*) so outbound DMs do not depend on the relay - /// also propagating an underlying chat_id; for everything else we send to the originating - /// chat via its oc_* chat_id, which Lark accepts uniformly for groups, threads, and - /// channels. + /// Builds the typed receive-target for a Lark inbound captured at agent creation. /// - /// If the inbound is p2p but the relay omitted SenderId, returning a typed pair would - /// silently re-create the original /daily 400 (typing the user open_id as chat_id). - /// Instead, return an empty typed pair with FellBackToPrefixInference=true so - /// falls back to the legacy prefix path and call sites emit a Debug - /// breadcrumb. The relay always emits Sender.PlatformId in production, so this path - /// is defensive. + /// + /// p2p (DM): prefer the tenant-stable union_id (on_*) when the relay + /// surfaces it. union_id is cross-app safe within the tenant — Lark accepts it as a + /// receive_id_type=union_id target regardless of whether the relay-side ingress app + /// matches the customer's outbound app. Without union_id we fall back to the sender + /// open_id (ou_*), which is app-scoped and produces + /// code:99992361 open_id cross app when the two apps differ; the fallback flips + /// FellBackToPrefixInference=true so the call site emits a Debug breadcrumb and + /// operators can correlate Lark rejections with missing-union_id ingress. + /// + /// + /// + /// group / channel / thread: prefer the inbound Lark chat_id (oc_*) which + /// is tenant-scoped — any app added to the chat can address it via + /// receive_id_type=chat_id. Without an explicit Lark chat_id the helper falls back to + /// the routing , which works only when the routing id is + /// itself a Lark oc_*; otherwise the outbound proxy will surface a Lark validation + /// failure that the call site logs and retries. + /// + /// + /// + /// If the inbound is p2p but the relay omitted both union_id and senderId, + /// returning a typed pair would silently re-create the original /daily 400 (typing the user + /// open_id as chat_id). Instead, return an empty typed pair with + /// FellBackToPrefixInference=true so falls back to the legacy + /// prefix path and call sites emit a Debug breadcrumb. + /// /// - public static LarkReceiveTarget BuildFromInbound(string? chatType, string? conversationId, string? senderId) + public static LarkReceiveTarget BuildFromInbound( + string? chatType, + string? conversationId, + string? senderId, + string? larkUnionId = null, + string? larkChatId = null) { - var trimmedSender = (senderId ?? string.Empty).Trim(); if (IsDirectMessage(chatType)) { - return string.IsNullOrEmpty(trimmedSender) - ? new LarkReceiveTarget(string.Empty, string.Empty, FellBackToPrefixInference: true) - : new LarkReceiveTarget(trimmedSender, OpenIdReceiveIdType, FellBackToPrefixInference: false); + // Cross-app safe: tenant-stable user identifier, accepted by any Lark app. + var trimmedUnion = (larkUnionId ?? string.Empty).Trim(); + if (!string.IsNullOrEmpty(trimmedUnion)) + return new LarkReceiveTarget(trimmedUnion, UnionIdReceiveIdType, FellBackToPrefixInference: false); + + // Fallback: app-scoped open_id. Will surface `code:99992361 open_id cross app` from + // Lark when the relay-side ingress app does not match the customer's outbound app. + // Flag the fallback so call sites can LogDebug for incident correlation. + var trimmedSender = (senderId ?? string.Empty).Trim(); + if (!string.IsNullOrEmpty(trimmedSender)) + return new LarkReceiveTarget(trimmedSender, OpenIdReceiveIdType, FellBackToPrefixInference: true); + + return new LarkReceiveTarget(string.Empty, string.Empty, FellBackToPrefixInference: true); } + // group / channel / thread: prefer the inbound Lark chat_id (cross-app within tenant). + var trimmedChat = (larkChatId ?? string.Empty).Trim(); + if (!string.IsNullOrEmpty(trimmedChat)) + return new LarkReceiveTarget(trimmedChat, DefaultReceiveIdType, FellBackToPrefixInference: false); + + // Fallback: assume the routing conversation_id is a Lark `oc_*` (legacy behavior pre + // ingress-side chat_id capture). If it is not, the proxy will reject and the call site + // logs the surfaced Lark error. var trimmedConversation = (conversationId ?? string.Empty).Trim(); return new LarkReceiveTarget(trimmedConversation, DefaultReceiveIdType, FellBackToPrefixInference: false); } diff --git a/agents/Aevatar.GAgents.ChannelRuntime/NyxRelayAgentBuilderFlow.cs b/agents/Aevatar.GAgents.ChannelRuntime/NyxRelayAgentBuilderFlow.cs index e35ba421f..f500344e7 100644 --- a/agents/Aevatar.GAgents.ChannelRuntime/NyxRelayAgentBuilderFlow.cs +++ b/agents/Aevatar.GAgents.ChannelRuntime/NyxRelayAgentBuilderFlow.cs @@ -64,8 +64,8 @@ public static MessageContent FormatToolResult(AgentBuilderFlowDecision decision, "create_daily_report" => FormatCreateDailyReportResult(doc.RootElement), "create_social_media" => TextContent(FormatCreateSocialMediaResult(doc.RootElement)), "list_templates" => TextContent(FormatListTemplatesResult(doc.RootElement)), - "list_agents" => TextContent(FormatListAgentsResult(doc.RootElement)), - "agent_status" => TextContent(FormatAgentStatusResult(doc.RootElement)), + "list_agents" => FormatListAgentsCard(doc.RootElement), + "agent_status" => FormatAgentStatusCard(doc.RootElement), "run_agent" => TextContent(FormatRunAgentResult(doc.RootElement)), "disable_agent" => TextContent(FormatLifecycleStatusResult("Agent disabled.", doc.RootElement)), "enable_agent" => TextContent(FormatLifecycleStatusResult("Agent enabled.", doc.RootElement)), @@ -366,6 +366,80 @@ private static string FormatListAgentsResult(JsonElement root) return string.Join('\n', lines); } + /// + /// Renders /agents as an interactive Lark card. Each agent gets a section block with + /// status fields and a "Status" button that triggers agent_builder_action=agent_status + /// (handled by ); a footer button cluster offers shortcuts + /// to create another agent or browse templates. Empty result keeps the existing helper-text + /// reply since there are no per-agent buttons to render. + /// + private static MessageContent FormatListAgentsCard(JsonElement root) + { + if (TryReadError(root, out var error)) + return TextContent($"List agents failed: {error}"); + + var content = new MessageContent(); + + if (!root.TryGetProperty("agents", out var agentsElement) || + agentsElement.ValueKind != JsonValueKind.Array || + agentsElement.GetArrayLength() == 0) + { + content.Cards.Add(new CardBlock + { + Kind = CardBlockKind.Section, + BlockId = "agents_empty", + Title = "No agents yet", + Text = "Create one with `/daily` for a daily GitHub report or `/social-media` for a social-media drafter.", + }); + content.Actions.Add(BuildButton("Create Daily Report", "open_daily_report_form", isPrimary: true)); + content.Actions.Add(BuildButton("Create Social Media", "open_social_media_form", isPrimary: false)); + return content; + } + + var summary = new CardBlock + { + Kind = CardBlockKind.Section, + BlockId = "agents_summary", + Title = "Your agents", + Text = "Tap **Status** under any agent to drill in. Action buttons there run, disable/enable, or delete the agent.", + }; + content.Cards.Add(summary); + + foreach (var item in agentsElement.EnumerateArray()) + { + var agentId = ReadString(item, "agent_id") ?? "unknown-agent"; + var template = ReadString(item, "template") ?? "unknown-template"; + var status = ReadString(item, "status") ?? "unknown"; + var nextRun = ReadString(item, "next_scheduled_run") ?? "pending"; + var lastRun = NormalizeOptional(ReadString(item, "last_run_at")); + + var card = new CardBlock + { + Kind = CardBlockKind.Section, + BlockId = $"agent_row:{agentId}", + Title = $"`{agentId}`", + Text = $"Template: `{template}` · Status: `{status}`\nNext run: `{nextRun}`{(lastRun is null ? string.Empty : $" · Last run: `{lastRun}`")}", + }; + content.Cards.Add(card); + + // Per-agent "Status" button: triggers `agent_status` action which AgentBuilderCardFlow + // already handles and re-renders as a status card with the run / lifecycle actions. + content.Actions.Add(BuildAgentScopedButton( + label: $"Status: {ShortenAgentId(agentId)}", + agentBuilderAction: "agent_status", + agentId: agentId, + isPrimary: false)); + } + + // Footer shortcut row mirrors what AgentBuilderCardFlow renders on the dedicated card + // path so users have one consistent UX whether they typed `/agents` or arrived via card. + content.Actions.Add(BuildButton("Create Daily Report", "open_daily_report_form", isPrimary: false)); + content.Actions.Add(BuildButton("Create Social Media", "open_social_media_form", isPrimary: false)); + content.Actions.Add(BuildButton("Templates", "list_templates", isPrimary: false)); + + return content; + } + private static string FormatAgentStatusResult(JsonElement root) { if (TryReadError(root, out var error)) @@ -385,6 +459,102 @@ private static string FormatAgentStatusResult(JsonElement root) $"Next commands: /run-agent {agentId}, /disable-agent {agentId}, /enable-agent {agentId}, /delete-agent {agentId} confirm"); } + /// + /// Renders /agent-status <agent_id> as an interactive card with action buttons + /// (Run, Disable, Enable, Delete). Each button submits the corresponding + /// agent_builder_action with the agent_id as an argument so + /// can route the click to the existing tool action without + /// the user having to retype the id. Mirrors the card produced by the card-flow path so the + /// text-command and card-flow surfaces stay visually consistent. + /// + private static MessageContent FormatAgentStatusCard(JsonElement root) + { + if (TryReadError(root, out var error)) + return TextContent($"Agent status failed: {error}"); + + var agentId = ReadString(root, "agent_id") ?? "unknown-agent"; + var template = ReadString(root, "template") ?? "unknown-template"; + var status = ReadString(root, "status") ?? "unknown"; + var schedule = $"{ReadString(root, "schedule_cron") ?? "n/a"} ({ReadString(root, "schedule_timezone") ?? "n/a"})"; + var lastRun = ReadString(root, "last_run_at") ?? "n/a"; + var nextRun = ReadString(root, "next_scheduled_run") ?? "n/a"; + var lastError = NormalizeOptional(ReadString(root, "last_error")); + var note = NormalizeOptional(ReadString(root, "note")); + + var bodyLines = new List + { + $"Agent ID: `{agentId}`", + $"Template: `{template}`", + $"Status: `{status}`", + $"Schedule: `{schedule}`", + $"Last run: `{lastRun}`", + $"Next run: `{nextRun}`", + }; + if (lastError is not null) + bodyLines.Add($"Last error: {lastError}"); + if (note is not null) + bodyLines.Add(note); + + var content = new MessageContent(); + content.Cards.Add(new CardBlock + { + Kind = CardBlockKind.Section, + BlockId = $"agent_status:{agentId}", + Title = "Agent Status", + Text = string.Join("\n", bodyLines), + }); + + // Lifecycle buttons mirror the legacy text "Next commands: ..." line. Disable and Enable + // are both shown so the user can flip status either direction without typing; the click + // handler enforces the invariants. Delete is marked danger so Lark renders it red and the + // user has a final visual confirm before submitting. + var isRunning = string.Equals(status, SkillRunnerDefaults.StatusRunning, StringComparison.OrdinalIgnoreCase) || + string.Equals(status, SkillRunnerDefaults.StatusError, StringComparison.OrdinalIgnoreCase); + content.Actions.Add(BuildAgentScopedButton("Run Now", "run_agent", agentId, isPrimary: isRunning)); + content.Actions.Add(BuildAgentScopedButton("Disable", "disable_agent", agentId, isPrimary: false)); + content.Actions.Add(BuildAgentScopedButton("Enable", "enable_agent", agentId, isPrimary: false)); + var deleteButton = BuildAgentScopedButton("Delete", "delete_agent", agentId, isPrimary: false); + deleteButton.IsDanger = true; + deleteButton.Arguments["confirm"] = "true"; + content.Actions.Add(deleteButton); + content.Actions.Add(BuildButton("Back to Agents", "list_agents", isPrimary: false)); + + return content; + } + + private static ActionElement BuildButton(string label, string agentBuilderAction, bool isPrimary) + { + var button = new ActionElement + { + Kind = ActionElementKind.Button, + ActionId = agentBuilderAction, + Label = label, + IsPrimary = isPrimary, + }; + button.Arguments["agent_builder_action"] = agentBuilderAction; + return button; + } + + private static ActionElement BuildAgentScopedButton(string label, string agentBuilderAction, string agentId, bool isPrimary) + { + var button = BuildButton(label, agentBuilderAction, isPrimary); + button.Arguments["agent_id"] = agentId; + return button; + } + + /// + /// Compresses long agent ids (e.g. skill-runner-94d754dfdfbb416aa5a676cecd0d7a71) into + /// a 10-char suffix so per-agent button labels stay readable in narrow Lark cards. The full + /// id is still carried in the button's arguments so the click handler routes correctly. + /// + private static string ShortenAgentId(string agentId) + { + if (string.IsNullOrEmpty(agentId) || agentId.Length <= 14) + return agentId; + + return $"…{agentId[^10..]}"; + } + private static string FormatRunAgentResult(JsonElement root) { if (TryReadError(root, out var error)) diff --git a/agents/Aevatar.GAgents.ChannelRuntime/SkillRunnerGAgent.cs b/agents/Aevatar.GAgents.ChannelRuntime/SkillRunnerGAgent.cs index 4ee9a9a87..60a7e78fa 100644 --- a/agents/Aevatar.GAgents.ChannelRuntime/SkillRunnerGAgent.cs +++ b/agents/Aevatar.GAgents.ChannelRuntime/SkillRunnerGAgent.cs @@ -301,13 +301,34 @@ private async Task SendOutputAsync(string output, CancellationToken ct) { // Surface downstream rejection so HandleTriggerAsync sees a real failure instead of // persisting SkillRunnerExecutionCompletedEvent on a silently-dropped Lark response. - throw new InvalidOperationException( - larkCode is { } code - ? $"Lark message delivery rejected (code={code}): {detail}" - : $"Lark message delivery rejected: {detail}"); + // The Error field on SkillRunnerExecutionFailedEvent ends up in `/agent-status`'s + // `last_error`, so for known recurring stale-state codes we expand the bare Lark + // message into actionable recovery guidance — otherwise the user sees a cryptic + // `99992361 open_id cross app` and has no way to know they need to rebuild the + // agent. + throw new InvalidOperationException(BuildLarkRejectionMessage(larkCode, detail)); } } + private static string BuildLarkRejectionMessage(int? larkCode, string detail) + { + if (larkCode == LarkBotErrorCodes.OpenIdCrossApp) + { + // The agent's persisted OutboundConfig was captured before union_id ingress existed + // (PR #409 added that), so `LarkReceiveIdType=open_id` is permanently scoped to a + // different Lark app than the customer outbound. Self-heal is not possible without + // a fresh ingress event carrying union_id; the user must rebuild the agent. + return + $"Lark message delivery rejected (code={larkCode}): {detail}. " + + "This agent was created before cross-app union_id ingress existed; " + + "delete and recreate it (`/agents` → Delete → `/daily`) to pick up the cross-app safe target."; + } + + return larkCode is { } code + ? $"Lark message delivery rejected (code={code}): {detail}" + : $"Lark message delivery rejected: {detail}"; + } + private async Task TrySendFailureAsync(string error, CancellationToken ct) { try { await SendOutputAsync($"Skill runner failed: {error}", ct); } diff --git a/agents/channels/Aevatar.GAgents.Channel.NyxIdRelay/NyxIdRelayTransport.cs b/agents/channels/Aevatar.GAgents.Channel.NyxIdRelay/NyxIdRelayTransport.cs index 25099a638..5a8c5afd6 100644 --- a/agents/channels/Aevatar.GAgents.Channel.NyxIdRelay/NyxIdRelayTransport.cs +++ b/agents/channels/Aevatar.GAgents.Channel.NyxIdRelay/NyxIdRelayTransport.cs @@ -127,6 +127,8 @@ public NyxIdRelayParseResult Parse(byte[] bodyBytes) NyxPlatform = platform, NyxConversationId = payload.Conversation?.Id?.Trim() ?? conversationIdentity, NyxPlatformMessageId = platformMessageId, + NyxLarkUnionId = ExtractLarkUnionId(platform, payload, isCardAction), + NyxLarkChatId = ExtractLarkChatId(platform, payload, isCardAction), }, }; @@ -308,6 +310,84 @@ private static string ResolveConversationIdentity(string platform, NyxIdRelayCal return $"{platform}-conversation"; } + /// + /// Extracts the Lark union_id (on_*) of the inbound sender from the relay's + /// raw_platform_data. union_id is tenant-stable and cross-app safe — outbound + /// senders running under a different Lark app than the relay-side ingress app must use this + /// to avoid open_id cross app rejections from Lark. Returns empty when the platform + /// is not Lark or the original event did not surface a union_id at the well-known + /// path. The empty case is normal for non-Lark traffic and for misconfigured Lark apps that + /// have not enabled union_id emission. + /// + private static string ExtractLarkUnionId(string platform, NyxIdRelayCallbackPayload payload, bool isCardAction) + { + if (!IsLark(platform) || payload.RawPlatformData is not { } raw || raw.ValueKind != JsonValueKind.Object) + return string.Empty; + + if (!raw.TryGetProperty("event", out var evt) || evt.ValueKind != JsonValueKind.Object) + return string.Empty; + + // Lark `im.message.receive_v1` puts sender ids under `event.sender.sender_id`. Card + // submissions (`card.action.trigger`) put the operator's union_id directly under + // `event.operator`, since there is no `sender_id` envelope on that event shape. + if (isCardAction) + { + if (evt.TryGetProperty("operator", out var op) && op.ValueKind == JsonValueKind.Object) + return ReadStringProperty(op, "union_id"); + return string.Empty; + } + + if (!evt.TryGetProperty("sender", out var sender) || sender.ValueKind != JsonValueKind.Object) + return string.Empty; + if (!sender.TryGetProperty("sender_id", out var senderId) || senderId.ValueKind != JsonValueKind.Object) + return string.Empty; + + return ReadStringProperty(senderId, "union_id"); + } + + /// + /// Extracts the Lark chat_id (oc_*) of the inbound conversation from the + /// relay's raw_platform_data. Cross-app safe within the tenant for groups/threads/ + /// channels — any app added to the chat can address it via receive_id_type=chat_id. + /// For p2p DMs the chat_id is bot-specific (each Lark app has its own DM thread with the + /// user) and not cross-app safe; downstream senders must prefer + /// for p2p targets. Returns empty when the platform is not Lark or the event did not carry + /// a chat_id at the well-known path. + /// + private static string ExtractLarkChatId(string platform, NyxIdRelayCallbackPayload payload, bool isCardAction) + { + if (!IsLark(platform) || payload.RawPlatformData is not { } raw || raw.ValueKind != JsonValueKind.Object) + return string.Empty; + + if (!raw.TryGetProperty("event", out var evt) || evt.ValueKind != JsonValueKind.Object) + return string.Empty; + + if (isCardAction) + { + if (evt.TryGetProperty("context", out var ctx) && ctx.ValueKind == JsonValueKind.Object) + return ReadStringProperty(ctx, "open_chat_id"); + return string.Empty; + } + + if (!evt.TryGetProperty("message", out var message) || message.ValueKind != JsonValueKind.Object) + return string.Empty; + + return ReadStringProperty(message, "chat_id"); + } + + private static bool IsLark(string platform) => + string.Equals(platform, "lark", StringComparison.OrdinalIgnoreCase) || + string.Equals(platform, "feishu", StringComparison.OrdinalIgnoreCase); + + private static string ReadStringProperty(JsonElement element, string propertyName) + { + if (!element.TryGetProperty(propertyName, out var property) || property.ValueKind != JsonValueKind.String) + return string.Empty; + + var value = property.GetString(); + return string.IsNullOrWhiteSpace(value) ? string.Empty : value.Trim(); + } + private static string BuildCanonicalKey( string platform, ConversationScope scope, diff --git a/test/Aevatar.GAgents.ChannelRuntime.Tests/AgentBuilderCardFlowTests.cs b/test/Aevatar.GAgents.ChannelRuntime.Tests/AgentBuilderCardFlowTests.cs index 6e69c2fbc..8e5cb07a3 100644 --- a/test/Aevatar.GAgents.ChannelRuntime.Tests/AgentBuilderCardFlowTests.cs +++ b/test/Aevatar.GAgents.ChannelRuntime.Tests/AgentBuilderCardFlowTests.cs @@ -38,6 +38,30 @@ public async Task TryResolveAsync_DailyReportLaunch_PrefillsSavedGithubUsername( decision.ReplyContent.Cards.Single().Text.Should().Contain("already filled in"); } + [Fact] + public async Task TryResolveAsync_TemplatesCardButton_DispatchesListTemplatesTool() + { + // The /agents card surfaces a `Templates` button; PR #409 added it. Without an explicit + // case in the card_action switch the button click would no-op and confuse users who + // navigate by tapping rather than typing /templates. Pin the contract so a refactor can + // not silently drop the routing. + var inbound = new ChannelInboundEvent + { + ChatType = "card_action", + RegistrationScopeId = "scope-1", + }; + inbound.Extra["agent_builder_action"] = "list_templates"; + + var decision = await AgentBuilderCardFlow.TryResolveAsync(inbound, userConfigQueryPort: null); + + decision.Should().NotBeNull(); + decision!.RequiresToolExecution.Should().BeTrue(); + decision.ToolAction.Should().Be("list_templates"); + + using var body = JsonDocument.Parse(decision.ToolArgumentsJson!); + body.RootElement.GetProperty("action").GetString().Should().Be("list_templates"); + } + [Fact] public async Task TryResolveAsync_DailyReportSubmit_AllowsMissingGithubUsername_ForUserConfigFallback() { diff --git a/test/Aevatar.GAgents.ChannelRuntime.Tests/AgentBuilderToolTests.cs b/test/Aevatar.GAgents.ChannelRuntime.Tests/AgentBuilderToolTests.cs index aa3fa6f26..a71cae4bf 100644 --- a/test/Aevatar.GAgents.ChannelRuntime.Tests/AgentBuilderToolTests.cs +++ b/test/Aevatar.GAgents.ChannelRuntime.Tests/AgentBuilderToolTests.cs @@ -11,6 +11,7 @@ using Aevatar.Studio.Application.Studio.Abstractions; using FluentAssertions; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; using NSubstitute; using Xunit; using StudioUserConfig = Aevatar.Studio.Application.Studio.Abstractions.UserConfig; @@ -198,9 +199,10 @@ await skillRunnerActor.Received(1).HandleEventAsync( e.Payload.Unpack().OutboundConfig.NyxApiKey == "full-key-1" && e.Payload.Unpack().OutboundConfig.ApiKeyId == "key-1" && e.Payload.Unpack().OutboundConfig.OwnerNyxUserId == "user-1" && - // For p2p the typed delivery target pins the user open_id from SenderId so - // outbound DMs go through Lark's open_id receive type even when the relay's - // ConversationId is the underlying oc_* chat or a route id. + // p2p inbound without LarkUnionId in the request context falls back to the + // sender open_id. Lark accepts this only when the relay-side and outbound + // apps match; cross-app deployments must populate LarkUnionId at ingress + // (see test below) to avoid `code:99992361 open_id cross app` rejections. e.Payload.Unpack().OutboundConfig.LarkReceiveId == "ou_user_1" && e.Payload.Unpack().OutboundConfig.LarkReceiveIdType == "open_id"), Arg.Any()); @@ -228,6 +230,326 @@ await skillRunnerActor.Received(1).HandleEventAsync( } } + [Fact] + public async Task ExecuteAsync_CreateAgent_DailyReport_PinsLarkUnionId_When_RelayPropagatesIt() + { + // Cross-app outbound delivery (`code:99992361 open_id cross app`) requires the + // tenant-stable `union_id`. When the relay surfaces it via + // ChannelMetadataKeys.LarkUnionId the typed delivery target on + // InitializeSkillRunnerCommand must pin (union_id, "union_id") instead of falling back + // to the relay-app-scoped open_id. Integration counterpart of + // LarkConversationTargetsTests.BuildFromInbound_ShouldPreferLarkUnionId_*. + var queryPort = Substitute.For(); + queryPort.GetStateVersionAsync("skill-runner-union-1", Arg.Any()) + .Returns(Task.FromResult(null), Task.FromResult(1)); + queryPort.GetAsync("skill-runner-union-1", Arg.Any()) + .Returns(Task.FromResult(new UserAgentCatalogEntry + { + AgentId = "skill-runner-union-1", + AgentType = SkillRunnerDefaults.AgentType, + TemplateName = "daily_report", + Status = SkillRunnerDefaults.StatusRunning, + })); + + var skillRunnerActor = Substitute.For(); + skillRunnerActor.Id.Returns("skill-runner-union-1"); + + var actorRuntime = Substitute.For(); + actorRuntime.GetAsync("skill-runner-union-1").Returns(Task.FromResult(null)); + actorRuntime.CreateAsync("skill-runner-union-1", Arg.Any()) + .Returns(Task.FromResult(skillRunnerActor)); + + var handler = new RoutingJsonHandler(); + handler.Add(HttpMethod.Get, "/api/v1/users/me", """{"user":{"id":"user-1"}}"""); + handler.Add(HttpMethod.Get, "/api/v1/providers/my-tokens", """ + { + "tokens": [ + { + "provider_id":"provider-github", + "provider_name":"GitHub", + "provider_slug":"github", + "provider_type":"oauth2", + "status":"active", + "connected_at":"2026-04-15T00:00:00Z" + } + ] + } + """); + handler.Add(HttpMethod.Get, "/api/v1/proxy/services?per_page=100", """ + { + "services": [ + {"id":"svc-github","slug":"api-github"} + ], + "custom_services": [ + {"id":"svc-lark","slug":"api-lark-bot"} + ], + "total": 2, + "page": 1, + "per_page": 100 + } + """); + handler.Add(HttpMethod.Post, "/api/v1/api-keys", """{"id":"key-union-1","full_key":"full-key-union-1"}"""); + + var nyxClient = new NyxIdApiClient( + new NyxIdToolOptions { BaseUrl = "https://nyx.example.com" }, + new HttpClient(handler) { BaseAddress = new Uri("https://nyx.example.com") }); + + var services = new ServiceCollection(); + services.AddSingleton(queryPort); + services.AddSingleton(actorRuntime); + services.AddSingleton(nyxClient); + var tool = new AgentBuilderTool(services.BuildServiceProvider()); + + AgentToolRequestContext.CurrentMetadata = new Dictionary + { + [LLMRequestMetadataKeys.NyxIdAccessToken] = "session-token", + [ChannelMetadataKeys.ChatType] = "p2p", + [ChannelMetadataKeys.ConversationId] = "oc_dm_chat_1", + [ChannelMetadataKeys.SenderId] = "ou_user_1", + [ChannelMetadataKeys.LarkUnionId] = "on_user_1", + [ChannelMetadataKeys.LarkChatId] = "oc_dm_chat_1", + ["scope_id"] = "scope-1", + }; + try + { + var result = await tool.ExecuteAsync(""" + { + "action": "create_agent", + "template": "daily_report", + "agent_id": "skill-runner-union-1", + "github_username": "alice", + "schedule_cron": "0 9 * * *", + "schedule_timezone": "UTC" + } + """); + + using var doc = JsonDocument.Parse(result); + doc.RootElement.GetProperty("status").GetString().Should().Be("created"); + + await skillRunnerActor.Received(1).HandleEventAsync( + Arg.Is(e => + e.Payload != null && + e.Payload.Is(InitializeSkillRunnerCommand.Descriptor) && + e.Payload.Unpack().OutboundConfig.LarkReceiveId == "on_user_1" && + e.Payload.Unpack().OutboundConfig.LarkReceiveIdType == "union_id"), + Arg.Any()); + } + finally + { + AgentToolRequestContext.CurrentMetadata = null; + } + } + + [Fact] + public async Task ExecuteAsync_CreateAgent_DailyReport_LogsFallbackBreadcrumb_When_LarkUnionIdMissing() + { + // Reviewer (PR #409 r3141562097): when the relay does not surface LarkUnionId at agent + // creation, BuildFromInbound returns (ou_*, open_id, FellBack=true). The flag itself is + // not persisted on OutboundConfig (typed receive id/type only), so a downstream + // LarkConversationTargets.Resolve() at SkillRunner send time sees populated typed fields + // and reports FellBack=false — meaning the cross-app risk is invisible to operators + // unless the agent-create site logs it once. Pin the LogDebug breadcrumb so the + // observability promised in the PR description actually fires in production. + var queryPort = Substitute.For(); + queryPort.GetStateVersionAsync("skill-runner-fallback-1", Arg.Any()) + .Returns(Task.FromResult(null), Task.FromResult(1)); + queryPort.GetAsync("skill-runner-fallback-1", Arg.Any()) + .Returns(Task.FromResult(new UserAgentCatalogEntry + { + AgentId = "skill-runner-fallback-1", + AgentType = SkillRunnerDefaults.AgentType, + TemplateName = "daily_report", + Status = SkillRunnerDefaults.StatusRunning, + })); + + var skillRunnerActor = Substitute.For(); + skillRunnerActor.Id.Returns("skill-runner-fallback-1"); + + var actorRuntime = Substitute.For(); + actorRuntime.GetAsync("skill-runner-fallback-1").Returns(Task.FromResult(null)); + actorRuntime.CreateAsync("skill-runner-fallback-1", Arg.Any()) + .Returns(Task.FromResult(skillRunnerActor)); + + var handler = new RoutingJsonHandler(); + handler.Add(HttpMethod.Get, "/api/v1/users/me", """{"user":{"id":"user-1"}}"""); + handler.Add(HttpMethod.Get, "/api/v1/providers/my-tokens", """ + { + "tokens": [ + { + "provider_id":"provider-github", + "provider_name":"GitHub", + "provider_slug":"github", + "provider_type":"oauth2", + "status":"active", + "connected_at":"2026-04-15T00:00:00Z" + } + ] + } + """); + handler.Add(HttpMethod.Get, "/api/v1/proxy/services?per_page=100", """ + { + "services": [{"id":"svc-github","slug":"api-github"}], + "custom_services": [{"id":"svc-lark","slug":"api-lark-bot"}], + "total": 2, + "page": 1, + "per_page": 100 + } + """); + handler.Add(HttpMethod.Post, "/api/v1/api-keys", """{"id":"key-fallback-1","full_key":"full-key-fallback-1"}"""); + + var nyxClient = new NyxIdApiClient( + new NyxIdToolOptions { BaseUrl = "https://nyx.example.com" }, + new HttpClient(handler) { BaseAddress = new Uri("https://nyx.example.com") }); + + var services = new ServiceCollection(); + services.AddSingleton(queryPort); + services.AddSingleton(actorRuntime); + services.AddSingleton(nyxClient); + + var logger = new ListLogger(); + var tool = new AgentBuilderTool(services.BuildServiceProvider(), logger); + + AgentToolRequestContext.CurrentMetadata = new Dictionary + { + [LLMRequestMetadataKeys.NyxIdAccessToken] = "session-token", + [ChannelMetadataKeys.ChatType] = "p2p", + [ChannelMetadataKeys.ConversationId] = "oc_chat_1", + [ChannelMetadataKeys.SenderId] = "ou_user_1", + // Deliberately NO LarkUnionId / LarkChatId — this is the cross-app risky path. + ["scope_id"] = "scope-1", + }; + try + { + var result = await tool.ExecuteAsync(""" + { + "action": "create_agent", + "template": "daily_report", + "agent_id": "skill-runner-fallback-1", + "github_username": "alice", + "schedule_cron": "0 9 * * *", + "schedule_timezone": "UTC" + } + """); + + using var doc = JsonDocument.Parse(result); + doc.RootElement.GetProperty("status").GetString().Should().Be("created"); + + // The breadcrumb must capture enough context to correlate with downstream Lark + // `99992361` rejections: agent_id, the missing typed fields, and the chosen receive + // type. Otherwise operators get no signal and the silent-default bug class re-opens. + var fallback = logger.Entries.Should().ContainSingle(entry => + entry.Level == LogLevel.Debug && + entry.Message.Contains("Agent builder fell back to legacy delivery target inference") && + entry.Message.Contains("skill-runner-fallback-1") && + entry.Message.Contains("hasUnionId=False") && + entry.Message.Contains("hasLarkChatId=False") && + entry.Message.Contains("hasSenderId=True") && + entry.Message.Contains("resolvedReceiveIdType=open_id")).Subject; + fallback.Message.Should().Contain("99992361"); + } + finally + { + AgentToolRequestContext.CurrentMetadata = null; + } + } + + [Fact] + public async Task ExecuteAsync_CreateAgent_DailyReport_DoesNotLogFallback_When_LarkUnionIdPresent() + { + // Counterpart to the breadcrumb test: when the relay surfaces union_id, the typed + // delivery target is cross-app safe and we must NOT spam Debug logs on every successful + // ingress (otherwise the breadcrumb signal becomes useless noise once /agents traffic + // ramps up). + var queryPort = Substitute.For(); + queryPort.GetStateVersionAsync("skill-runner-no-fallback-1", Arg.Any()) + .Returns(Task.FromResult(null), Task.FromResult(1)); + queryPort.GetAsync("skill-runner-no-fallback-1", Arg.Any()) + .Returns(Task.FromResult(new UserAgentCatalogEntry + { + AgentId = "skill-runner-no-fallback-1", + AgentType = SkillRunnerDefaults.AgentType, + TemplateName = "daily_report", + Status = SkillRunnerDefaults.StatusRunning, + })); + + var skillRunnerActor = Substitute.For(); + skillRunnerActor.Id.Returns("skill-runner-no-fallback-1"); + + var actorRuntime = Substitute.For(); + actorRuntime.GetAsync("skill-runner-no-fallback-1").Returns(Task.FromResult(null)); + actorRuntime.CreateAsync("skill-runner-no-fallback-1", Arg.Any()) + .Returns(Task.FromResult(skillRunnerActor)); + + var handler = new RoutingJsonHandler(); + handler.Add(HttpMethod.Get, "/api/v1/users/me", """{"user":{"id":"user-1"}}"""); + handler.Add(HttpMethod.Get, "/api/v1/providers/my-tokens", """ + { + "tokens": [ + { + "provider_id":"provider-github", + "provider_slug":"github", + "provider_type":"oauth2", + "status":"active", + "connected_at":"2026-04-15T00:00:00Z" + } + ] + } + """); + handler.Add(HttpMethod.Get, "/api/v1/proxy/services?per_page=100", """ + { + "services": [{"id":"svc-github","slug":"api-github"}], + "custom_services": [{"id":"svc-lark","slug":"api-lark-bot"}], + "total": 2, + "page": 1, + "per_page": 100 + } + """); + handler.Add(HttpMethod.Post, "/api/v1/api-keys", """{"id":"key-no-fallback-1","full_key":"full-key-no-fallback-1"}"""); + + var nyxClient = new NyxIdApiClient( + new NyxIdToolOptions { BaseUrl = "https://nyx.example.com" }, + new HttpClient(handler) { BaseAddress = new Uri("https://nyx.example.com") }); + + var services = new ServiceCollection(); + services.AddSingleton(queryPort); + services.AddSingleton(actorRuntime); + services.AddSingleton(nyxClient); + + var logger = new ListLogger(); + var tool = new AgentBuilderTool(services.BuildServiceProvider(), logger); + + AgentToolRequestContext.CurrentMetadata = new Dictionary + { + [LLMRequestMetadataKeys.NyxIdAccessToken] = "session-token", + [ChannelMetadataKeys.ChatType] = "p2p", + [ChannelMetadataKeys.ConversationId] = "oc_chat_1", + [ChannelMetadataKeys.SenderId] = "ou_user_1", + [ChannelMetadataKeys.LarkUnionId] = "on_user_1", + [ChannelMetadataKeys.LarkChatId] = "oc_chat_1", + ["scope_id"] = "scope-1", + }; + try + { + await tool.ExecuteAsync(""" + { + "action": "create_agent", + "template": "daily_report", + "agent_id": "skill-runner-no-fallback-1", + "github_username": "alice", + "schedule_cron": "0 9 * * *", + "schedule_timezone": "UTC" + } + """); + + logger.Entries.Should().NotContain(entry => + entry.Message.Contains("fell back to legacy delivery target inference")); + } + finally + { + AgentToolRequestContext.CurrentMetadata = null; + } + } + [Fact] public async Task ExecuteAsync_CreateAgent_DailyReport_UsesSavedGithubUsernamePreference_WhenArgumentMissing() { @@ -1547,4 +1869,38 @@ public Task SaveGithubUsernameAsync(string scopeId, string githubUsername, Cance return Task.CompletedTask; } } + + /// + /// Minimal in-memory that records each log call so tests can assert + /// on level + formatted message. Avoids a full Microsoft.Extensions.Logging.Testing dependency + /// for a single observability assertion. + /// + private sealed class ListLogger : ILogger + { + public List Entries { get; } = new(); + + public IDisposable BeginScope(TState state) where TState : notnull => NullScope.Instance; + + public bool IsEnabled(LogLevel logLevel) => true; + + public void Log( + LogLevel logLevel, + EventId eventId, + TState state, + Exception? exception, + Func formatter) + { + ArgumentNullException.ThrowIfNull(formatter); + Entries.Add(new LogEntry(logLevel, formatter(state, exception))); + } + + public sealed record LogEntry(LogLevel Level, string Message); + + private sealed class NullScope : IDisposable + { + public static readonly NullScope Instance = new(); + + public void Dispose() { } + } + } } diff --git a/test/Aevatar.GAgents.ChannelRuntime.Tests/LarkConversationTargetsTests.cs b/test/Aevatar.GAgents.ChannelRuntime.Tests/LarkConversationTargetsTests.cs index f4ba28fb9..8b1bac06a 100644 --- a/test/Aevatar.GAgents.ChannelRuntime.Tests/LarkConversationTargetsTests.cs +++ b/test/Aevatar.GAgents.ChannelRuntime.Tests/LarkConversationTargetsTests.cs @@ -90,13 +90,33 @@ public void Resolve_ShouldFallBackToChatIdDefault_WhenLegacyConversationIdIsUnre } [Fact] - public void BuildFromInbound_ShouldUseSenderOpenId_ForP2pDirectMessages() + public void BuildFromInbound_ShouldPreferLarkUnionId_ForP2pDirectMessages() { - // The relay puts the Lark sender open_id (`ou_*`) into ChannelInboundEvent.SenderId, - // while ConversationId may be the route id or the DM's underlying `oc_*` chat_id — - // neither of which Lark's outbound API will accept with receive_id_type=chat_id when - // the bot is only authorised to DM the user, not the synthetic DM thread. Pin sender - // open_id at delivery-target creation time. + // union_id is tenant-stable and cross-app safe, so a relay-side ingress app and an + // outbound-side customer app see the SAME `on_*` for the user. This eliminates the + // `code:99992361 open_id cross app` rejection that was breaking SkillRunner DMs when + // only the open_id was available. + var target = LarkConversationTargets.BuildFromInbound( + chatType: "p2p", + conversationId: "oc_dm_underlying_chat", + senderId: "ou_user_1", + larkUnionId: "on_user_1", + larkChatId: "oc_dm_underlying_chat"); + + target.ReceiveId.Should().Be("on_user_1"); + target.ReceiveIdType.Should().Be("union_id"); + target.FellBackToPrefixInference.Should().BeFalse(); + } + + [Fact] + public void BuildFromInbound_ShouldFallBackToSenderOpenId_ForP2pWithoutUnionId() + { + // When the relay does not surface a union_id (older relay revisions, misconfigured + // Lark app, non-Lark traffic mistyped as p2p), the only DM identifier we have is the + // sender open_id. Lark accepts it inside the originating app and rejects it + // (`code:99992361 open_id cross app`) when sent from a different app — flip + // FellBackToPrefixInference=true so call sites LogDebug and operators can correlate + // a missing-union_id ingress with downstream Lark rejections. var target = LarkConversationTargets.BuildFromInbound( chatType: "p2p", conversationId: "oc_dm_underlying_chat", @@ -104,6 +124,25 @@ public void BuildFromInbound_ShouldUseSenderOpenId_ForP2pDirectMessages() target.ReceiveId.Should().Be("ou_user_1"); target.ReceiveIdType.Should().Be("open_id"); + target.FellBackToPrefixInference.Should().BeTrue(); + } + + [Fact] + public void BuildFromInbound_ShouldPreferLarkChatId_ForGroupChats() + { + // For groups/channels/threads, the inbound Lark `oc_*` chat_id is tenant-scoped and + // cross-app safe (any app added to the chat can post via receive_id_type=chat_id). + // Prefer it over the routing conversation_id, which may be a NyxID-internal route id + // that Lark cannot resolve. + var target = LarkConversationTargets.BuildFromInbound( + chatType: "group", + conversationId: "ddd-1234-route-id", + senderId: "ou_user_1", + larkUnionId: "on_user_1", + larkChatId: "oc_group_chat_1"); + + target.ReceiveId.Should().Be("oc_group_chat_1"); + target.ReceiveIdType.Should().Be("chat_id"); target.FellBackToPrefixInference.Should().BeFalse(); } diff --git a/test/Aevatar.GAgents.ChannelRuntime.Tests/NyxIdRelayTransportTests.cs b/test/Aevatar.GAgents.ChannelRuntime.Tests/NyxIdRelayTransportTests.cs index 960b99ac9..5d4d26270 100644 --- a/test/Aevatar.GAgents.ChannelRuntime.Tests/NyxIdRelayTransportTests.cs +++ b/test/Aevatar.GAgents.ChannelRuntime.Tests/NyxIdRelayTransportTests.cs @@ -358,4 +358,146 @@ public void Parse_ShouldReportIgnored_ForCardActionWithNonJsonText() parsed.Ignored.Should().BeTrue(); parsed.ErrorCode.Should().Be("invalid_card_action_payload"); } + + [Fact] + public void Parse_ShouldExposeLarkUnionIdAndChatId_FromMessageReceiveRawPlatformData() + { + // Cross-app outbound delivery (issue: `code:99992361 open_id cross app`) needs the + // tenant-stable `union_id` and the inbound `chat_id` to be visible at the agent-builder + // layer. Both live inside the original Lark `im.message.receive_v1` event payload that + // NyxID forwards verbatim under `raw_platform_data`. + var body = """ + { + "message_id": "msg-lark-1", + "platform": "lark", + "agent": { "api_key_id": "api-key-1" }, + "conversation": { "id": "conv-1", "platform_id": "oc_chat_1", "type": "private" }, + "sender": { "platform_id": "ou_user_1", "display_name": "User One" }, + "content": { "type": "text", "text": "/daily" }, + "raw_platform_data": { + "schema": "2.0", + "header": { "event_type": "im.message.receive_v1" }, + "event": { + "sender": { + "sender_id": { + "open_id": "ou_user_1", + "union_id": "on_user_1", + "user_id": "u123" + } + }, + "message": { + "message_id": "om_lark_1", + "chat_id": "oc_dm_chat_1", + "chat_type": "p2p" + } + } + } + } + """; + + var parsed = _transport.Parse(Encoding.UTF8.GetBytes(body)); + + parsed.Success.Should().BeTrue(); + parsed.Activity!.TransportExtras.NyxLarkUnionId.Should().Be("on_user_1"); + parsed.Activity.TransportExtras.NyxLarkChatId.Should().Be("oc_dm_chat_1"); + } + + [Fact] + public void Parse_ShouldExposeLarkUnionIdAndChatId_FromCardActionRawPlatformData() + { + // Lark card submissions arrive as `card.action.trigger`, which carries the operator's + // identifiers under `event.operator` (no `sender_id` envelope) and the chat under + // `event.context.open_chat_id`. Verify both are surfaced on TransportExtras so the + // post-submit agent-builder branch can consume the cross-app safe pair. + var body = """ + { + "message_id": "msg-card-1", + "platform": "lark", + "agent": { "api_key_id": "api-key-1" }, + "conversation": { "id": "conv-2", "platform_id": "oc_chat_2", "type": "private" }, + "sender": { "platform_id": "ou_user_2", "display_name": "User Two" }, + "content": { + "content_type": "card_action", + "text": "{\"value\":{\"agent_builder_action\":\"create_daily_report\"}}" + }, + "raw_platform_data": { + "schema": "2.0", + "header": { "event_type": "card.action.trigger" }, + "event": { + "operator": { + "open_id": "ou_user_2", + "union_id": "on_user_2", + "user_id": "u456" + }, + "context": { + "open_chat_id": "oc_dm_chat_2", + "open_message_id": "om_card_2" + }, + "action": { + "tag": "button", + "value": { "agent_builder_action": "create_daily_report" } + } + } + } + } + """; + + var parsed = _transport.Parse(Encoding.UTF8.GetBytes(body)); + + parsed.Success.Should().BeTrue(); + parsed.Activity!.TransportExtras.NyxLarkUnionId.Should().Be("on_user_2"); + parsed.Activity.TransportExtras.NyxLarkChatId.Should().Be("oc_dm_chat_2"); + } + + [Fact] + public void Parse_ShouldLeaveLarkExtrasEmpty_ForNonLarkPlatform() + { + // The lark_* TransportExtras fields are intentionally Lark-specific: other platforms + // populate their own platform-native equivalents. Pin the empty contract for non-Lark + // traffic so a future refactor that broadens the parser cannot accidentally write + // Lark IDs onto Telegram / Discord activities. + var body = """ + { + "message_id": "msg-tg-1", + "platform": "telegram", + "agent": { "api_key_id": "api-key-1" }, + "conversation": { "id": "conv-tg-1", "platform_id": "12345", "type": "private" }, + "sender": { "platform_id": "tg-user-1", "display_name": "User TG" }, + "content": { "type": "text", "text": "hi" }, + "raw_platform_data": { + "event": { + "sender": { "sender_id": { "union_id": "should_not_propagate" } }, + "message": { "chat_id": "should_not_propagate" } + } + } + } + """; + + var parsed = _transport.Parse(Encoding.UTF8.GetBytes(body)); + + parsed.Success.Should().BeTrue(); + parsed.Activity!.TransportExtras.NyxLarkUnionId.Should().BeEmpty(); + parsed.Activity.TransportExtras.NyxLarkChatId.Should().BeEmpty(); + } + + [Fact] + public void Parse_ShouldLeaveLarkExtrasEmpty_WhenRawPlatformDataMissing() + { + var body = """ + { + "message_id": "msg-lark-2", + "platform": "lark", + "agent": { "api_key_id": "api-key-1" }, + "conversation": { "id": "conv-3", "platform_id": "oc_chat_3", "type": "private" }, + "sender": { "platform_id": "ou_user_3", "display_name": "User Three" }, + "content": { "type": "text", "text": "/daily" } + } + """; + + var parsed = _transport.Parse(Encoding.UTF8.GetBytes(body)); + + parsed.Success.Should().BeTrue(); + parsed.Activity!.TransportExtras.NyxLarkUnionId.Should().BeEmpty(); + parsed.Activity.TransportExtras.NyxLarkChatId.Should().BeEmpty(); + } } diff --git a/test/Aevatar.GAgents.ChannelRuntime.Tests/NyxRelayAgentBuilderFlowTests.cs b/test/Aevatar.GAgents.ChannelRuntime.Tests/NyxRelayAgentBuilderFlowTests.cs index 49ad7180f..a29a03225 100644 --- a/test/Aevatar.GAgents.ChannelRuntime.Tests/NyxRelayAgentBuilderFlowTests.cs +++ b/test/Aevatar.GAgents.ChannelRuntime.Tests/NyxRelayAgentBuilderFlowTests.cs @@ -152,8 +152,12 @@ public void TryResolve_ShouldBuildCreateSocialMediaToolCall_FromTextCommand() } [Fact] - public void FormatToolResult_ShouldRenderPlainTextListAgentsResponse() + public void FormatToolResult_ShouldRenderListAgentsAsInteractiveCard() { + // /agents now returns an interactive card so users can drill into per-agent status + // without retyping the long agent_id. Per-row "Status" buttons carry the full agent_id + // in their arguments so AgentBuilderCardFlow's existing card_action handler routes them + // back to the agent_status tool path. var decision = AgentBuilderFlowDecision.ToolCall("list_agents", """{"action":"list_agents"}"""); var result = NyxRelayAgentBuilderFlow.FormatToolResult( decision, @@ -161,20 +165,93 @@ public void FormatToolResult_ShouldRenderPlainTextListAgentsResponse() { "agents": [ { - "agent_id": "agent-1", + "agent_id": "skill-runner-94d754dfdfbb416aa5a676cecd0d7a71", "template": "daily_report", "status": "running", - "next_scheduled_run": "2026-04-23T09:00:00Z" + "next_scheduled_run": "2026-04-23T09:00:00Z", + "last_run_at": "2026-04-22T09:00:00Z" } ] } """); - result.Actions.Should().BeEmpty(); + result.Cards.Should().NotBeEmpty(); + // First card is the summary; subsequent cards are per-agent rows. + result.Cards[0].Title.Should().Be("Your agents"); + result.Cards.Skip(1).Should().ContainSingle(card => + card.BlockId == "agent_row:skill-runner-94d754dfdfbb416aa5a676cecd0d7a71"); + + var statusButton = result.Actions.Should().Contain(a => a.ActionId == "agent_status").Subject; + statusButton.Arguments.Should().Contain(new KeyValuePair( + "agent_builder_action", "agent_status")); + statusButton.Arguments.Should().Contain(new KeyValuePair( + "agent_id", "skill-runner-94d754dfdfbb416aa5a676cecd0d7a71")); + + result.Actions.Should().Contain(a => a.ActionId == "open_daily_report_form"); + result.Actions.Should().Contain(a => a.ActionId == "open_social_media_form"); + } + + [Fact] + public void FormatToolResult_ShouldRenderEmptyListAgentsAsCallToActionCard() + { + var decision = AgentBuilderFlowDecision.ToolCall("list_agents", """{"action":"list_agents"}"""); + var result = NyxRelayAgentBuilderFlow.FormatToolResult(decision, """{"agents":[]}"""); + + result.Cards.Should().ContainSingle(card => card.BlockId == "agents_empty"); + result.Actions.Should().Contain(a => a.ActionId == "open_daily_report_form"); + result.Actions.Should().Contain(a => a.ActionId == "open_social_media_form"); + } + + [Fact] + public void FormatToolResult_ShouldRenderAgentStatusAsInteractiveCard_WithLifecycleButtons() + { + // /agent-status now ships as an interactive card with one button per lifecycle action + // (Run, Disable, Enable, Delete) so the user does not have to retype the agent_id for + // each follow-up command. Each button carries the agent_id in its arguments and the + // delete button additionally carries `confirm=true` so AgentBuilderCardFlow's existing + // confirm-required handler skips the second-step prompt. + var decision = AgentBuilderFlowDecision.ToolCall("agent_status", """{"action":"agent_status"}"""); + var result = NyxRelayAgentBuilderFlow.FormatToolResult( + decision, + """ + { + "agent_id": "skill-runner-1", + "template": "daily_report", + "status": "error", + "schedule_cron": "0 9 * * *", + "schedule_timezone": "UTC", + "last_run_at": "2026-04-25T05:30:00Z", + "next_scheduled_run": "2026-04-26T09:00:00Z", + "last_error": "Lark message delivery rejected" + } + """); + + result.Cards.Should().ContainSingle(card => card.BlockId == "agent_status:skill-runner-1"); + result.Cards[0].Text.Should().Contain("Status: `error`"); + result.Cards[0].Text.Should().Contain("Last error:"); + + result.Actions.Should().Contain(a => a.ActionId == "run_agent"); + result.Actions.Should().Contain(a => a.ActionId == "disable_agent"); + result.Actions.Should().Contain(a => a.ActionId == "enable_agent"); + result.Actions.Should().Contain(a => a.ActionId == "list_agents"); + + var deleteButton = result.Actions.Should().Contain(a => a.ActionId == "delete_agent").Subject; + deleteButton.IsDanger.Should().BeTrue(); + deleteButton.Arguments.Should().Contain(new KeyValuePair("confirm", "true")); + deleteButton.Arguments.Should().Contain(new KeyValuePair("agent_id", "skill-runner-1")); + } + + [Fact] + public void FormatToolResult_ShouldRenderAgentStatusError_WhenToolReturnsError() + { + var decision = AgentBuilderFlowDecision.ToolCall("agent_status", """{"action":"agent_status"}"""); + var result = NyxRelayAgentBuilderFlow.FormatToolResult( + decision, + """{"error":"Agent not found"}"""); + result.Cards.Should().BeEmpty(); - result.Text.Should().Contain("Current agents:"); - result.Text.Should().Contain("agent-1: template=daily_report, status=running"); - result.Text.Should().Contain("/agent-status "); + result.Actions.Should().BeEmpty(); + result.Text.Should().Contain("Agent status failed: Agent not found"); } [Fact] diff --git a/test/Aevatar.GAgents.ChannelRuntime.Tests/SkillRunnerGAgentTests.cs b/test/Aevatar.GAgents.ChannelRuntime.Tests/SkillRunnerGAgentTests.cs index ed8c52ece..ba1e5824d 100644 --- a/test/Aevatar.GAgents.ChannelRuntime.Tests/SkillRunnerGAgentTests.cs +++ b/test/Aevatar.GAgents.ChannelRuntime.Tests/SkillRunnerGAgentTests.cs @@ -203,6 +203,42 @@ await act.Should().ThrowAsync() .WithMessage("*upstream timeout*"); } + [Fact] + public async Task SendOutputAsync_ShouldIncludeRecreateHint_When_LarkRejectsAsCrossAppOpenId() + { + // PR #409 review (pulls/409#review-4175198266): after this fix new agents capture + // union_id, but agents created before the fix still have `LarkReceiveIdType=open_id` + // pinned to a relay-app-scoped `ou_*`. Their next scheduled run hits Lark + // `99992361 open_id cross app` and the user sees the bare error in `/agent-status`'s + // `last_error` with no clue what to do. Surface explicit "delete and recreate" guidance + // so the failure becomes self-documenting. + var initialize = CreateInitializeCommand(); + initialize.OutboundConfig = new SkillRunnerOutboundConfig + { + ConversationId = "oc_chat_1", + NyxProviderSlug = "api-lark-bot", + NyxApiKey = "nyx-api-key", + LarkReceiveId = "ou_relay_app_user_1", + LarkReceiveIdType = "open_id", + }; + await _agent.HandleInitializeAsync(initialize); + + var handler = new RecordingHandler( + """{"code":99992361,"msg":"open_id cross app","error":{"message":"Refer to the documentation"}}"""); + AttachNyxIdApiClient(_agent, handler); + + Func act = () => InvokeSendOutputAsync(_agent, "report"); + + var assertion = await act.Should().ThrowAsync(); + assertion.WithMessage("*code=99992361*"); + assertion.WithMessage("*open_id cross app*"); + // The hint must be actionable enough that the user can recover without reading source. + assertion.WithMessage("*before cross-app union_id ingress existed*"); + assertion.WithMessage("*/agents*"); + assertion.WithMessage("*Delete*"); + assertion.WithMessage("*/daily*"); + } + private static void AttachNyxIdApiClient(SkillRunnerGAgent agent, HttpMessageHandler handler) { var client = new NyxIdApiClient(