diff --git a/agents/Aevatar.GAgents.ChannelRuntime/AgentBuilderTool.cs b/agents/Aevatar.GAgents.ChannelRuntime/AgentBuilderTool.cs index 57d364378..b429a4c49 100644 --- a/agents/Aevatar.GAgents.ChannelRuntime/AgentBuilderTool.cs +++ b/agents/Aevatar.GAgents.ChannelRuntime/AgentBuilderTool.cs @@ -233,20 +233,10 @@ private async Task CreateDailyReportAgentAsync( ? SkillRunnerDefaults.GenerateActorId() : args.Str("agent_id")!.Trim(); - // Diagnostic (#417 follow-up, PR for production 403 root-cause hunt): log the resolved - // service-id list and the exact payload we're about to send to NyxID `POST /api-keys`. - // The api-key request body is otherwise opaque in production; without this we can't tell - // whether the deployed binary actually carries the `allow_all_services=false` field, or - // whether `requiredServiceIds` resolved to per-user `UserService.id`s vs catalog ids. - var apiKeyPayload = BuildCreateApiKeyPayload(agentId, requiredServiceIds.value!); - _logger?.LogInformation( - "Diagnostic[#417]: minting agent api-key. agentId={AgentId} requiredSlugs={Slugs} resolvedServiceIds={Ids} payload={Payload}", - agentId, - string.Join(",", templateSpec!.RequiredServiceSlugs), - string.Join(",", requiredServiceIds.value!), - apiKeyPayload); - - var createKeyResponse = await nyxClient.CreateApiKeyAsync(token, apiKeyPayload, ct); + var createKeyResponse = await nyxClient.CreateApiKeyAsync( + token, + BuildCreateApiKeyPayload(agentId, requiredServiceIds.value!), + ct); if (IsErrorPayload(createKeyResponse)) return createKeyResponse; @@ -254,30 +244,19 @@ private async Task CreateDailyReportAgentAsync( if (!TryParseApiKeyCreateResponse(createKeyResponse, out var apiKeyId, out var apiKeyValue, out var apiKeyError)) return JsonSerializer.Serialize(new { error = apiKeyError }); - _logger?.LogInformation( - "Diagnostic[#417]: agent api-key created. agentId={AgentId} apiKeyId={ApiKeyId}", - agentId, - apiKeyId); - - // Issue aevatarAI/aevatar#411 / #417 follow-up: catch in-flight GitHub OAuth revocation. - // The earlier `BuildGitHubAuthorizationResponseAsync` check covers the "no provider token - // at all" case; this preflight only earns its keep when the OAuth grant was revoked or - // had its scopes downgraded between connect-time and create-time. + // Issue aevatarAI/aevatar#411 / #417 follow-up: catch in-flight GitHub-side issues. + // The earlier `BuildGitHubAuthorizationResponseAsync` check covers the "no provider + // token at all" case; this preflight catches misconfigurations that only surface at + // request time (the original case under #421 was a missing `User-Agent` header that + // GitHub rejects with 403; OAuth grant revocation is the other one). // - // Codex review (PR #418 r3141846175): even though the api-key itself is correctly - // configured under #417, we still revoke it on preflight failure. Reason: each retry of - // `/daily` mints a *new* api-key before the preflight runs, so without revoke the user's - // NyxID account accumulates one orphan proxy-scoped key per failed retry until they - // re-authorize GitHub. The revoke is a best-effort cleanup, not a safety claim about the - // key's correctness. + // PR #418 review r3141846175: revoke the freshly-minted key on preflight failure so + // each `/daily` retry doesn't leave another orphan proxy-scoped key behind in the + // user's NyxID account. The revoke is best-effort cleanup, not a safety claim about + // the key's correctness. var preflight = await PreflightGitHubProxyAsync(nyxClient, apiKeyValue!, providerSlug, ct); if (preflight is not null) { - _logger?.LogWarning( - "Diagnostic[#417]: preflight failed; revoking new api-key. agentId={AgentId} apiKeyId={ApiKeyId} preflightResponse={Response}", - agentId, - apiKeyId, - preflight); await BestEffortRevokeApiKeyAsync(nyxClient, token, apiKeyId!, "github_preflight_failed", ct); return preflight; } @@ -1778,18 +1757,6 @@ private static string NormalizeScopeId(string? value) => extraHeaders: null, ct); - // Diagnostic (#417 follow-up): log the *raw* probe response. The parsed JSON envelope - // we return as `proxy_body` only carries the inner Lark/GitHub `body` string when - // SendAsync wraps non-2xx; the unparsed `probe` is the only place we see what NyxID - // actually returned. Distinguishes: - // - `{"error":true,"status":403,"error_code":9000,"message":"API key does not have access..."}` → NyxID `ApiKeyScopeForbidden` (our payload still wrong) - // - `{"error":true,"status":403,"body":"{\"message\":\"Bad credentials\",...}"}` → GitHub upstream 403 (OAuth grant revoked) - // - other shapes → unclassified - _logger?.LogInformation( - "Diagnostic[#417]: GitHub preflight probe response. providerSlug={ProviderSlug} probe={Probe}", - nyxProviderSlug, - string.IsNullOrWhiteSpace(probe) ? "" : probe); - if (string.IsNullOrWhiteSpace(probe)) return null; @@ -1833,7 +1800,7 @@ private static string NormalizeScopeId(string? value) => detail = string.IsNullOrWhiteSpace(detail) ? "GitHub proxy returned 401/403 for the new agent API key." : detail, http_status = status, proxy_body = string.IsNullOrWhiteSpace(body) ? null : body, - hint = "GitHub returned 401/403 to NyxID's stored OAuth token. The token was likely revoked at GitHub or had its scopes downgraded after the provider was connected. Re-authorize the GitHub provider at NyxID, then run /daily again.", + hint = "GitHub returned 401/403 through the NyxID proxy. Common causes: (a) the OAuth grant for GitHub was revoked at github.com/settings/applications or its scopes were downgraded — re-authorize the GitHub provider at NyxID; (b) the request reached GitHub without a User-Agent header (NyxIdApiClient now sends a default; if you see this, check that the deployed binary includes that fix). The agent will not produce a useful daily report until proxy access succeeds.", nyx_provider_slug = nyxProviderSlug, }); } diff --git a/src/Aevatar.AI.ToolProviders.NyxId/NyxIdApiClient.cs b/src/Aevatar.AI.ToolProviders.NyxId/NyxIdApiClient.cs index 5d78d0239..0e23f81e6 100644 --- a/src/Aevatar.AI.ToolProviders.NyxId/NyxIdApiClient.cs +++ b/src/Aevatar.AI.ToolProviders.NyxId/NyxIdApiClient.cs @@ -23,6 +23,19 @@ public sealed record NyxIdChannelRelayReplyResult( /// HTTP client for calling NyxID REST API endpoints. public sealed class NyxIdApiClient { + /// + /// Default User-Agent injected on every call to + /// when the caller does not specify one in extraHeaders. GitHub's REST API rejects + /// requests without a User-Agent with HTTP 403 ("Request forbidden by administrative + /// rules") — see https://docs.github.com/en/rest/overview/resources-in-the-rest-api#user-agent-required. + /// .NET's HttpClient does not set one by default; NyxID proxies the client's headers + /// through to GitHub, so the absence at the .NET layer manifests as a GitHub 403 in + /// production. CLI tools written against reqwest (e.g. nyxid proxy request) + /// happen to send reqwest/x.y as their default and so never hit this. + /// + public const string DefaultProxyUserAgent = "aevatar-agent-builder"; + private const string UserAgentHeaderName = "User-Agent"; + private readonly HttpClient _http; private readonly NyxIdToolOptions _options; private readonly ILogger _logger; @@ -131,12 +144,26 @@ public async Task ProxyRequestAsync( using var request = new HttpRequestMessage(httpMethod, url); request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", token); + var callerSpecifiedUserAgent = false; if (extraHeaders != null) { foreach (var (key, value) in extraHeaders) + { request.Headers.TryAddWithoutValidation(key, value); + if (string.Equals(key, UserAgentHeaderName, StringComparison.OrdinalIgnoreCase)) + callerSpecifiedUserAgent = true; + } } + // GitHub-required User-Agent (#417 follow-up). NyxID proxies whatever the .NET client + // sends, and HttpClient sends none by default, so without this every GitHub call lands + // as 403 "Request forbidden by administrative rules". Inject a default for *all* proxy + // targets — non-GitHub services don't care about UA either way, and pinning it at the + // proxy boundary means SkillRunner / agent-builder / preflight all benefit without + // every call site remembering to pass it. + if (!callerSpecifiedUserAgent) + request.Headers.TryAddWithoutValidation(UserAgentHeaderName, DefaultProxyUserAgent); + if (!string.IsNullOrEmpty(body) && httpMethod != HttpMethod.Get && httpMethod != HttpMethod.Head) { request.Content = new StringContent(body, Encoding.UTF8, "application/json"); diff --git a/test/Aevatar.AI.Tests/NyxIdApiClientCoverageTests.cs b/test/Aevatar.AI.Tests/NyxIdApiClientCoverageTests.cs index 40f0374ea..caf3856dc 100644 --- a/test/Aevatar.AI.Tests/NyxIdApiClientCoverageTests.cs +++ b/test/Aevatar.AI.Tests/NyxIdApiClientCoverageTests.cs @@ -175,6 +175,66 @@ public async Task UpdateChannelRelayTextReplyAsync_ShouldRejectEmptyText() result.Should().BeEquivalentTo(new NyxIdChannelRelayReplyResult(false, Detail: "missing_reply_text")); } + [Fact] + public async Task ProxyRequestAsync_ShouldInjectDefaultUserAgent_WhenCallerOmitsIt() + { + // #417 follow-up: GitHub's REST API rejects requests without a `User-Agent` header + // with a 403 "Request forbidden by administrative rules". .NET's `HttpClient` doesn't + // send one by default, and NyxID proxies whatever the .NET client sends — so without + // this default, every agent-builder GitHub call lands as a spurious 403 (root cause of + // production /daily failures captured under PR #420 diagnostic logs). + var handler = new CaptureHandler(new HttpResponseMessage(HttpStatusCode.OK) + { + Content = new StringContent("{}", Encoding.UTF8, "application/json"), + }); + var client = new NyxIdApiClient( + new NyxIdToolOptions { BaseUrl = "https://nyx.example.com" }, + new HttpClient(handler), + NullLogger.Instance); + + await client.ProxyRequestAsync( + "token", + "api-github", + "/rate_limit", + "GET", + body: null, + extraHeaders: null, + CancellationToken.None); + + handler.LastRequest.Should().NotBeNull(); + var ua = handler.LastRequest!.Headers.UserAgent.ToString(); + ua.Should().Be(NyxIdApiClient.DefaultProxyUserAgent); + } + + [Fact] + public async Task ProxyRequestAsync_ShouldHonorCallerSuppliedUserAgent() + { + // The default is only injected when the caller doesn't specify one — agent code that + // wants to identify as a different UA (e.g. SkillRunner with a per-template label) + // should win over the boundary default. + var handler = new CaptureHandler(new HttpResponseMessage(HttpStatusCode.OK) + { + Content = new StringContent("{}", Encoding.UTF8, "application/json"), + }); + var client = new NyxIdApiClient( + new NyxIdToolOptions { BaseUrl = "https://nyx.example.com" }, + new HttpClient(handler), + NullLogger.Instance); + + await client.ProxyRequestAsync( + "token", + "api-github", + "/rate_limit", + "GET", + body: null, + extraHeaders: new Dictionary { ["User-Agent"] = "custom-skill-runner/1.2.3" }, + CancellationToken.None); + + handler.LastRequest.Should().NotBeNull(); + var ua = handler.LastRequest!.Headers.UserAgent.ToString(); + ua.Should().Be("custom-skill-runner/1.2.3"); + } + [Fact] public void TryParseErrorEnvelope_ShouldHandleEmptyInvalidAndStructuredResponses() { diff --git a/test/Aevatar.GAgents.ChannelRuntime.Tests/AgentBuilderToolTests.cs b/test/Aevatar.GAgents.ChannelRuntime.Tests/AgentBuilderToolTests.cs index 70248112b..7d81bdeb6 100644 --- a/test/Aevatar.GAgents.ChannelRuntime.Tests/AgentBuilderToolTests.cs +++ b/test/Aevatar.GAgents.ChannelRuntime.Tests/AgentBuilderToolTests.cs @@ -434,7 +434,10 @@ public async Task ExecuteAsync_CreateAgent_DailyReport_FailsClosed_When_GithubPr doc.RootElement.GetProperty("http_status").GetInt32().Should().Be(403); // The hint should point users at re-authorizing the GitHub provider at NyxID, not // at api-key bindings (which used to be the misdiagnosis under #411 — see #417). - doc.RootElement.GetProperty("hint").GetString().Should().Contain("Re-authorize"); + // Match case-insensitively so future hint copy edits (capitalization, punctuation) + // don't require flipping this assertion in lockstep — the *intent* of the assertion + // is "hint mentions re-authorization", not "hint matches one specific prefix". + doc.RootElement.GetProperty("hint").GetString()!.ToLowerInvariant().Should().Contain("re-authorize"); // The actor must NOT receive InitializeSkillRunnerCommand — preflight aborts // BEFORE the actor is invoked so we don't leave a broken agent in the catalog.