Add outbound network diagnostics scaffolding#24801
Conversation
5fc55d2 to
ee112a0
Compare
ee112a0 to
beb17da
Compare
a673d13 to
53d4eab
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. 🚀 ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
fcoury-oai
left a comment
There was a problem hiding this comment.
Manually verified opt-in OAuth network diagnostics: forced a proxied token-exchange failure, confirmed redacted diagnostic logs appear only when enabled, are captured with default login logging, and do not expose sensitive values.
Codex review had one suggestion that I am leaving as optional.
Other than that code looks good. Approved 👍
| if rendered.contains("tls") || rendered.contains("certificate") || rendered.contains("cert") { | ||
| return RouteFailureClass::TlsError; | ||
| } | ||
| if error.is_connect() { |
There was a problem hiding this comment.
Suggestion by Codex:
is_connect() is broader than DNS resolution, so proxy connection refusals and other connection-stage failures will be emitted as resolver_error. Could we add a generic connection failure class, or fall back to other, unless we have evidence that resolution itself failed?
|
Closed in favor of #26706 , first in a series of stacked PRs for PAC functionality. |
Summary
[network]proxy config surface and schema entries, with project-local overrides deniedCODEX_NETWORK_DIAGNOSTICS=1Diagnostic behavior
When
CODEX_NETWORK_DIAGNOSTICS=1is set, auth flows log only presence bits for proxy/CA env vars,CODEX_SYSTEM_PROXYstate, coarse HTTP/transport failure classes, and status codes. Proxy values, URLs, headers, bodies, tokens, CA paths, and credentials are not logged. With the env var unset, the new diagnostic helpers are silent.