Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
134 changes: 112 additions & 22 deletions agents/Aevatar.GAgents.Authoring.Lark/AgentBuilderTool.cs
Original file line number Diff line number Diff line change
Expand Up @@ -266,17 +266,30 @@ private async Task<string> CreateDailyReportAgentAsync(
return gitHubAuthorizationResponse;

var providerSlug = (args.Str("nyx_provider_slug") ?? "api-lark-bot").Trim();
var requiredServiceIds = await ResolveProxyServiceIdsAsync(nyxClient, token, templateSpec!.RequiredServiceSlugs, ct);
if (requiredServiceIds.errorJson != null)
return requiredServiceIds.errorJson;
var serviceResolution = await ResolveProxyServiceIdsAsync(nyxClient, token, templateSpec!.RequiredServiceSlugs, ct);
if (serviceResolution.ErrorJson != null)
return serviceResolution.ErrorJson;

// Issue #423 §C — capture the inbound channel-bot slug as a failure-notification
// fallback. By definition the user can be reached through the bot they just
// messaged, so when a primary outbound delivery is rejected (e.g. cross-tenant
// Lark `99992364`) the failure-notification message can still land if the agent's
// API key is allowed to route through the inbound bot. Optional: if the inbound
// slug is not registered as a per-user UserService row (or equals the primary,
// in which case the fallback would just hit the same proxy), we leave the field
// empty and TrySendFailureAsync degrades to the current single-attempt behavior.
var failureNotificationContext = ResolveFailureNotificationContext(
providerSlug,
serviceResolution.RequiredIds!,
serviceResolution.EligibleIdBySlug);

var agentId = string.IsNullOrWhiteSpace(args.Str("agent_id"))
? SkillRunnerDefaults.GenerateActorId()
: args.Str("agent_id")!.Trim();

var createKeyResponse = await nyxClient.CreateApiKeyAsync(
token,
BuildCreateApiKeyPayload(agentId, requiredServiceIds.value!),
BuildCreateApiKeyPayload(agentId, failureNotificationContext.AllowedServiceIds),
ct);

if (IsErrorPayload(createKeyResponse))
Expand Down Expand Up @@ -328,6 +341,7 @@ private async Task<string> CreateDailyReportAgentAsync(
LarkReceiveIdFallback = deliveryTarget.Fallback?.ReceiveId ?? string.Empty,
LarkReceiveIdTypeFallback = deliveryTarget.Fallback?.ReceiveIdType ?? string.Empty,
OwnerScope = caller.Clone(),
FailureNotificationProviderSlug = failureNotificationContext.FailureSlug ?? string.Empty,
};
#pragma warning restore CS0612

Expand Down Expand Up @@ -445,17 +459,17 @@ private async Task<string> CreateSocialMediaAgentAsync(
// hardcoded `[providerSlug, publishProviderSlug]` was fine for two slugs but would
// drift if a third slug were ever added; route through the spec so the source of
// truth lives next to the workflow YAML.
var requiredServiceIds = await ResolveProxyServiceIdsAsync(
var serviceResolution = await ResolveProxyServiceIdsAsync(
nyxClient,
token,
templateSpec!.RequiredServiceSlugs,
ct);
if (requiredServiceIds.errorJson != null)
return requiredServiceIds.errorJson;
if (serviceResolution.ErrorJson != null)
return serviceResolution.ErrorJson;

var createKeyResponse = await nyxClient.CreateApiKeyAsync(
token,
BuildCreateApiKeyPayload(agentId, requiredServiceIds.value!),
BuildCreateApiKeyPayload(agentId, serviceResolution.RequiredIds!),
ct);

if (IsErrorPayload(createKeyResponse))
Expand Down Expand Up @@ -1046,29 +1060,45 @@ private static bool SupportsManagedLifecycle(string? agentType) =>
/// slug, and skip org-shared rows the caller cannot use as a proxy target — those would later
/// surface as a less-actionable <c>org_role_insufficient</c> error.</para>
/// </remarks>
private async Task<(IReadOnlyList<string>? value, string? errorJson)> ResolveProxyServiceIdsAsync(
/// <summary>
/// Result of <see cref="ResolveProxyServiceIdsAsync"/>. <see cref="RequiredIds"/> /
/// <see cref="ErrorJson"/> are mutually exclusive (success vs. blocking error). Even on
/// success, callers can use <see cref="EligibleIdBySlug"/> to look up <em>optional</em>
/// slugs that were not in <c>requiredSlugs</c> — e.g. the inbound channel-bot slug for
/// SkillRunner's failure-notification fallback (issue #423 §C). Optional lookups must
/// not block agent creation, so they go through this map instead of being added to
/// <c>requiredSlugs</c> (which would cause <see cref="ResolveProxyServiceIdsAsync"/> to
/// return a <c>service_not_connected</c> error if the slug is missing).
/// </summary>
private readonly record struct ProxyServiceResolutionResult(
IReadOnlyList<string>? RequiredIds,
string? ErrorJson,
IReadOnlyDictionary<string, string> EligibleIdBySlug);

private async Task<ProxyServiceResolutionResult> ResolveProxyServiceIdsAsync(
NyxIdApiClient client,
string token,
IReadOnlyList<string> requiredSlugs,
CancellationToken ct)
{
var emptyEligible = new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase);
if (requiredSlugs.Count == 0)
{
return (null, JsonSerializer.Serialize(new
return new ProxyServiceResolutionResult(null, JsonSerializer.Serialize(new
{
error = "no_required_slugs",
hint = "At least one required Nyx proxy service slug must be provided.",
}));
}), emptyEligible);
}

var response = await client.ListUserServicesAsync(token, ct);
if (IsErrorPayload(response))
{
return (null, JsonSerializer.Serialize(new
return new ProxyServiceResolutionResult(null, JsonSerializer.Serialize(new
{
error = "user_services_unavailable",
hint = "Could not list connected Nyx user-services. Try again or check NyxID availability.",
}));
}), emptyEligible);
}

try
Expand Down Expand Up @@ -1126,17 +1156,29 @@ private static bool SupportsManagedLifecycle(string? agentType) =>
bestBySlug[slug] = candidate;
}

// Snapshot the eligible (slug → id) map before the per-required-slug check so
// callers can look up optional slugs (e.g. inbound channel-bot for failure-
// notification fallback) without re-listing user-services. Ineligible rows are
// intentionally excluded — including them would let optional lookups silently
// pick up an inactive or org-viewer-only service the API key cannot route through.
var eligibleBySlug = bestBySlug
.Where(static pair => pair.Value.IsEligible)
.ToDictionary(
pair => pair.Key,
pair => pair.Value.Id,
StringComparer.OrdinalIgnoreCase);

var ids = new List<string>(requiredSlugs.Count);
foreach (var slug in requiredSlugs.Distinct(StringComparer.OrdinalIgnoreCase))
{
if (!bestBySlug.TryGetValue(slug, out var resolution))
{
return (null, JsonSerializer.Serialize(new
return new ProxyServiceResolutionResult(null, JsonSerializer.Serialize(new
{
error = "service_not_connected",
slug,
hint = $"NyxID has no connected user-service for slug `{slug}`. Connect the provider at NyxID before creating this agent.",
}));
}), emptyEligible);
}

if (resolution.IsEligible)
Expand All @@ -1148,32 +1190,35 @@ private static bool SupportsManagedLifecycle(string? agentType) =>
if (string.Equals(resolution.CredentialSourceType, "org", StringComparison.OrdinalIgnoreCase) &&
resolution.OrgAllowed != true)
{
return (null, JsonSerializer.Serialize(new
return new ProxyServiceResolutionResult(null, JsonSerializer.Serialize(new
{
error = "service_org_viewer_only",
slug,
hint = $"NyxID user-service for slug `{slug}` is shared by your org but your role does not permit using it as a proxy target. Ask an admin to widen the org role scope, or connect a personal credential.",
}));
}), emptyEligible);
}

// Remaining ineligible reason: !is_active.
return (null, JsonSerializer.Serialize(new
return new ProxyServiceResolutionResult(null, JsonSerializer.Serialize(new
{
error = "service_inactive",
slug,
hint = $"NyxID user-service for slug `{slug}` is inactive. Re-activate it at NyxID before creating this agent.",
}));
}), emptyEligible);
}

return (ids.Distinct(StringComparer.Ordinal).ToArray(), null);
return new ProxyServiceResolutionResult(
ids.Distinct(StringComparer.Ordinal).ToArray(),
null,
eligibleBySlug);
}
catch (JsonException)
{
return (null, JsonSerializer.Serialize(new
return new ProxyServiceResolutionResult(null, JsonSerializer.Serialize(new
{
error = "user_services_parse_failed",
hint = "NyxID user-services response was not valid JSON.",
}));
}), emptyEligible);
}
}

Expand All @@ -1188,6 +1233,51 @@ private readonly record struct ServiceResolution(
!(string.Equals(CredentialSourceType, "org", StringComparison.OrdinalIgnoreCase) && OrgAllowed != true);
}

/// <summary>
/// Result of resolving the inbound channel-bot fallback used by SkillRunner's
/// failure-notification path (issue #423 §C). When the inbound slug is reachable
/// (registered + eligible + distinct from the primary), <see cref="FailureSlug"/>
/// is set and its corresponding <c>UserService.id</c> is appended to
/// <see cref="AllowedServiceIds"/> so the agent's API key can route through it
/// at runtime. Otherwise <see cref="FailureSlug"/> is null and the agent
/// degrades to the existing single-attempt failure notification.
/// </summary>
private readonly record struct FailureNotificationContext(
string? FailureSlug,
IReadOnlyList<string> AllowedServiceIds);

private FailureNotificationContext ResolveFailureNotificationContext(
string primarySlug,
IReadOnlyList<string> requiredIds,
IReadOnlyDictionary<string, string> eligibleIdBySlug)
{
var inboundSlug = AgentToolRequestContext.TryGet(ChannelMetadataKeys.InboundChannelBotProxySlug)?.Trim();
if (string.IsNullOrWhiteSpace(inboundSlug))
return new FailureNotificationContext(null, requiredIds);

// Same-proxy fallback gives no recovery benefit — a primary rejection at
// `slug=X` would also fail at `slug=X`. Skip the capture so TrySendFailureAsync
// doesn't pay the wasted POST and doesn't double-log the same rejection.
if (string.Equals(inboundSlug, primarySlug, StringComparison.Ordinal))
return new FailureNotificationContext(null, requiredIds);

// Optional slug must be a connected, eligible user-service for the API key to
// route through it. If it's not, leaving the failure-notification field empty
// keeps the runtime on the existing single-attempt path — better than persisting
// a slug whose every send would 403 at proxy enforcement time.
if (!eligibleIdBySlug.TryGetValue(inboundSlug, out var inboundId))
return new FailureNotificationContext(null, requiredIds);

// Dedupe — if the inbound slug's UserService.id is already in requiredIds the
// expanded list is identical, but we still surface the slug on OutboundConfig so
// the runtime knows to use it for failure notifications.
var allowed = requiredIds.Contains(inboundId, StringComparer.Ordinal)
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This optional fallback can accidentally make /daily creation fail for org-shared inbound bots. /api/v1/user-services includes org-inherited rows as credential_source.type = org with allowed = true, so eligibleIdBySlug can return an org-owned UserService.id. But NyxID POST /api/v1/api-keys validates allowed_service_ids against the new key owner's own active UserService rows (user_id == key owner). If the required services are personal but the inbound channel-bot slug only resolves through an org-owned service, this append sends an org-owned id in allowed_service_ids and the key creation fails with “UserService ... not found or not owned by user”. That turns the optional failure-notification fallback into a creation blocker. Please either only capture personal rows for this optional slug, or create/scope the child key through a contract that can accept the org-owned id, and add a regression test with credential_source: { type: "org", allowed: true }.

? requiredIds
: requiredIds.Append(inboundId).ToArray();

return new FailureNotificationContext(inboundSlug, allowed);
}

private async Task<string?> BuildGitHubAuthorizationResponseAsync(
NyxIdApiClient client,
string token,
Expand Down
17 changes: 17 additions & 0 deletions agents/Aevatar.GAgents.Channel.Runtime/ChannelMetadataKeys.cs
Original file line number Diff line number Diff line change
Expand Up @@ -55,4 +55,21 @@ public static class ChannelMetadataKeys
/// <c>outbound_proxy_slug</c> form makes the routing-side semantics explicit.
/// </summary>
public const string LarkOutboundProxySlug = "channel.lark.outbound_proxy_slug";

/// <summary>
/// NyxID provider slug of the inbound channel-bot that received this turn's webhook
/// event. Equivalent to <c>ChannelInboundEvent.NyxProviderSlug</c>, surfaced as request
/// metadata so the agent-builder tool can capture it on the new agent's
/// <c>SkillRunnerOutboundConfig.FailureNotificationProviderSlug</c> at create time.
/// </summary>
/// <remarks>
/// The inbound channel-bot is the bot the user just successfully messaged. When the
/// agent's primary outbound proxy fails with a structural rejection (e.g. Lark
/// <c>99992364 user id cross tenant</c> from a cross-tenant relay/outbound mismatch),
/// the inbound bot's slug is the only known proxy that can still deliver to the user.
/// SkillRunner uses this for failure notifications only — primary outbound stays on the
/// caller-provided <c>nyx_provider_slug</c> argument so existing deployments are not
/// rerouted unexpectedly. See issue #423 §C.
/// </remarks>
public const string InboundChannelBotProxySlug = "channel.inbound.channel_bot_provider_slug";
}
Original file line number Diff line number Diff line change
Expand Up @@ -750,6 +750,13 @@ private static IReadOnlyDictionary<string, string> BuildReplyMetadata(
[ChannelMetadataKeys.ChatType] = inboundEvent.ChatType,
};

// Inbound channel-bot's NyxID provider slug. SkillRunner agent-builder captures this on
// SkillRunnerOutboundConfig.FailureNotificationProviderSlug so a failed outbound delivery
// (e.g. cross-tenant Lark 99992364) can still notify the user via the bot they just
// successfully messaged. See issue #423 §C and ChannelMetadataKeys.InboundChannelBotProxySlug.
if (!string.IsNullOrWhiteSpace(inboundEvent.NyxProviderSlug))
metadata[ChannelMetadataKeys.InboundChannelBotProxySlug] = inboundEvent.NyxProviderSlug;

var platformMessageId = NormalizeOptional(activity?.TransportExtras?.NyxPlatformMessageId);
if (!string.IsNullOrWhiteSpace(platformMessageId))
metadata[ChannelMetadataKeys.PlatformMessageId] = platformMessageId;
Expand Down
Loading
Loading