fix(nyxid-proxy): inject default User-Agent so GitHub doesn't 403

[eanzhao]

Closes #411.

Summary

Fixes the production /daily 403 — both at agent-create time (preflight) and at SkillRunner runtime (every nyxid_proxy GitHub call).

The smoking gun came from the diagnostic logs added in #420. GitHub returned 403 with body:

Request forbidden by administrative rules. Please make sure your request has a User-Agent header (https://docs.github.com/en/rest/overview/resources-in-the-rest-api#user-agent-required).

Not OAuth. Not ApiKeyScopeForbidden. GitHub rejects any REST request without a User-Agent header.

Why this only manifested in production

Caller	UA sent	GitHub response
nyxid proxy request ... (Rust reqwest)	reqwest/0.x.y (auto)	✅ 200
curl	curl/8.x (auto)	✅ 200
Production backend (.NET HttpClient)	none — HttpClient doesn't send one by default	❌ 403

Every CLI reproduction I did against the same NyxID account, same UserService, same api-key scope returned 200, because reqwest auto-injects a UA. The .NET HttpClient does not. NyxID's proxy transparently forwards whatever the client sends, so the missing-at-the-.NET-layer header surfaces as 403 Request forbidden by administrative rules from GitHub at the bottom of the stack.

This is also why all of #418's fixes were correct — allow_all_services=false, per-user UserService.ids, eligible-row picker, BestEffortRevoke — none of those had any effect on the GitHub-side rejection.

Why this fix closes #411

#411 describes the runtime symptom directly: daily_report agent gets created, then at SkillRunner execution time every GitHub call through nyxid_proxy returns 403. The runtime path is SkillRunnerGAgent → LLM tool calls nyxid_proxy → NyxIdProxyTool.ExecuteAsync (src/Aevatar.AI.ToolProviders.NyxId/Tools/NyxIdProxyTool.cs:111) → NyxIdApiClient.ProxyRequestAsync → NyxID → GitHub. By injecting the default UA at the ProxyRequestAsync boundary, every call on that runtime path now carries a UA and stops being rejected.

Mapping #411's suggested fixes to what landed:

#411 suggestion	Status
1. Preflight GitHub access at agent-create time	✅ Landed in #418 (PreflightGitHubProxyAsync in AgentBuilderTool.cs)
2. Surface "all GitHub calls failed at runtime" as SkillRunner failure with LastError	Not addressed here — root cause removed makes this a secondary concern. Can open a follow-up if anyone wants the bookkeeping anyway
3. Sanitized diagnostic logging for proxy 4xx bodies	Partial — github_proxy_access_denied envelope now includes the upstream proxy_body (carries the real GitHub error), and the temporary Diagnostic[#417] logs from #420 captured the production root cause that drove this PR (logs removed here, the diagnostic envelope stays)
4. Typed GitHub activity tool	Not addressed; orthogonal to the 403 root cause

Changes

• src/Aevatar.AI.ToolProviders.NyxId/NyxIdApiClient.cs:
  • Add DefaultProxyUserAgent constant (aevatar-agent-builder).
  • ProxyRequestAsync now injects this UA on every call when the caller hasn't supplied one. Caller-supplied User-Agent in extraHeaders (case-insensitive) wins.
  • Pinned at the proxy boundary so agent-builder preflight, SkillRunner runtime nyxid_proxy calls, and any future agent that reaches GitHub via NyxID proxy all benefit without each call site remembering to send UA.
• agents/Aevatar.GAgents.ChannelRuntime/AgentBuilderTool.cs:
  • Rewrite github_proxy_access_denied hint. The previous wording ("OAuth grant was revoked, re-authorize") was based on the fix(agent-builder): use UserService.id for api-key allowed_service_ids (#417) #418 misdiagnosis; the actual production failure was missing UA, not revoked OAuth. New hint covers both causes.
  • Remove the four Diagnostic[#417] log lines added in chore(agent-builder): diagnostic logs for /daily 403 root-cause hunt #420. They served their purpose and don't have a long-term place on the hot path (full payload JSON including api-key prefixes).
  • Tighten the comment block at the preflight call site to reflect the broader rationale (UA, OAuth revocation, etc.).
• test/Aevatar.AI.Tests/NyxIdApiClientCoverageTests.cs:
  • _ShouldInjectDefaultUserAgent_WhenCallerOmitsIt — pins the regression
  • _ShouldHonorCallerSuppliedUserAgent — pins that caller override still works
• test/Aevatar.GAgents.ChannelRuntime.Tests/AgentBuilderToolTests.cs:
  • Soften the hint substring assertion to a case-insensitive match (per review at 4175638754) so future hint copy edits don't require synchronized assertion flips.

Verification

• dotnet test test/Aevatar.AI.Tests/Aevatar.AI.Tests.csproj — 12/12 passing
• dotnet test test/Aevatar.GAgents.ChannelRuntime.Tests/... — 421/421 passing (verified at 160c474 after the review-driven assertion fix)
• Production smoke after merge: /daily alice from a Lark DM should now succeed end-to-end and the resulting agent's scheduled runs should produce a real GitHub-backed report.

Follow-up

Filed at the proxy layer too: ChronoAIProject/NyxID#514 proposes NyxID default-inject User-Agent for proxy targets whose upstream requires it. The contract "every NyxID proxy caller must remember to send UA" is fragile (Python urllib, Java HttpURLConnection, .NET HttpClient all default to no UA); the proxy is a better place to enforce GitHub's hard requirement than every SDK.

Related

• SkillRunner daily_report GitHub proxy 403s at runtime #411 — the runtime-failure issue this closes (root cause was layered: catalog-id-vs-UserService-id → allow_all_services default → User-Agent)
• fix(agent-builder): use UserService.id for api-key allowed_service_ids (#417) #418 — allow_all_services=false + per-user UserService.id fix (correct, but masked by the UA issue underneath)
• chore(agent-builder): diagnostic logs for /daily 403 root-cause hunt #420 — diagnostic logs that surfaced this; logs removed in this PR
• fix(lark): malformed v2 card schema causes nyx-api 502, /agents silently drops reply #416 / [codex] Fix Lark v2 card schema #419 — /agents 502 from malformed Lark v2 card payload (separate issue, fixed in [codex] Fix Lark v2 card schema #419)
• proxy: inject default User-Agent so GitHub-style "UA-required" services don't 403 silently ChronoAIProject/NyxID#514 — upstream proposal to default-inject UA at the proxy layer

[eanzhao]

I agree the root-cause hypothesis is plausible and matches the production-only shape: .NET HttpClient sends no default User-Agent, NyxID forwards caller headers, and NyxID's forward_request allowlist includes user-agent, so injecting the default at NyxIdApiClient.ProxyRequestAsync is the right boundary.

However, this PR is not merge-ready as-is: the local test command claimed in the PR body does not pass at head 2140fcb052f6ce9c67980abd1d16e8127c8204ce.

dotnet test test/Aevatar.GAgents.ChannelRuntime.Tests/Aevatar.GAgents.ChannelRuntime.Tests.csproj --nologo

Result: Failed: 1, Passed: 420, Total: 421.

Failing test:
AgentBuilderToolTests.ExecuteAsync_CreateAgent_DailyReport_FailsClosed_When_GithubProxyDeniedForNewKey

The new hint says re-authorize (lowercase), while the existing assertion still expects Re-authorize (uppercase). Please update the test assertion or the message so the stated 421/421 verification is true.

[eanzhao]

This changed hint currently breaks the existing ChannelRuntime test suite: ExecuteAsync_CreateAgent_DailyReport_FailsClosed_When_GithubProxyDeniedForNewKey still asserts the hint contains Re-authorize, but the new text contains lowercase re-authorize. I ran dotnet test test/Aevatar.GAgents.ChannelRuntime.Tests/Aevatar.GAgents.ChannelRuntime.Tests.csproj --nologo at 2140fcb052f6ce9c67980abd1d16e8127c8204ce; result was Failed: 1, Passed: 420, Total: 421. Please update either the assertion or the message before merge.

[eanzhao]

Fixed in 160c474.

You're right — I miscounted in the PR body. The new hint uses lowercase re-authorize while the existing assertion at AgentBuilderToolTests.cs:437 still pinned "Re-authorize", so the test was failing at 2140fcb0 (420/421).

Switched the assertion to a case-insensitive substring match (.ToLowerInvariant().Should().Contain("re-authorize")). The intent of the assertion is "hint still mentions re-authorization for the OAuth-revocation case", not "hint matches one specific capitalization" — case-pinning adds friction for future hint copy edits without strengthening the regression coverage.

Local verification at 160c474:

dotnet test test/Aevatar.GAgents.ChannelRuntime.Tests/Aevatar.GAgents.ChannelRuntime.Tests.csproj --nologo
Passed!  - Failed: 0, Passed: 421, Skipped: 0, Total: 421

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 70.39%. Comparing base (d359995) to head (160c474).
⚠️ Report is 4 commits behind head on dev.

@@            Coverage Diff             @@
##              dev     #421      +/-   ##
==========================================
+ Coverage   70.38%   70.39%   +0.01%     
==========================================
  Files        1175     1175              
  Lines       84453    84460       +7     
  Branches    11124    11126       +2     
==========================================
+ Hits        59440    59459      +19     
+ Misses      20721    20712       -9     
+ Partials     4292     4289       -3     

Flag	Coverage Δ
ci	70.39% <100.00%> (+0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
...c/Aevatar.AI.ToolProviders.NyxId/NyxIdApiClient.cs	68.99% <100.00%> (+2.04%)	⬆️

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.