Skip to content

feat: add CLI proxy support#219

Open
jlitola wants to merge 2 commits into
mainfrom
pinto-repair
Open

feat: add CLI proxy support#219
jlitola wants to merge 2 commits into
mainfrom
pinto-repair

Conversation

@jlitola

@jlitola jlitola commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Summary

  • add proxy-aware fetch for CLI/local MCP traffic honoring HTTP_PROXY, HTTPS_PROXY, and NO_PROXY
  • use native Node env proxy when the runtime explicitly supports the user opt-in, with an undici fallback for older supported runtimes
  • add real Node proxy smoke coverage, docs, Node engine updates, and MCP package metadata bump

Rationale

  • Node exposes native env proxy behavior only on newer 22.x/24.x runtimes and only after explicit opt-in; the CLI still supports Node 20, so we need a fallback implementation for proxy users on the supported floor.
  • The fallback uses undici because Node fetch is implemented by undici and undici provides ProxyAgent, letting us route proxied requests without mutating global fetch or the global dispatcher.
  • We use undici@7.28.0 because undici 7 supports Node >=20.18.1. undici 8 requires Node >=22.19.0, which would drop Node 20 entirely; undici 6 would let us keep the old 20.12 floor but would pin us to an older major after we already decided raising the Node 20 floor is acceptable.
  • The Node engine floor is raised from ^20.12.0 to ^20.18.1 to match undici 7's published engine requirement. The >=22.13.0 side remains unchanged.
  • @githits/mcp is bumped to 0.6.1 because its package metadata engine floor changed; without a package version bump, that metadata would not publish.

Proxy Test Coverage

  • Unit tests in src/services/proxy-fetch.test.ts cover env precedence, native-proxy runtime gates, malformed proxy URL redaction, transport-error redaction, HTTP_PROXY fallback for HTTPS targets, NO_PROXY exact/domain/subdomain/wildcard/IPv6/port behavior, and lazy fetch creation for update checks.
  • The pipeline-ready smoke command is bun run smoke:proxy-node. It builds src/services/proxy-fetch.ts with Bun, then runs a real Node child process against local HTTP, HTTPS, and proxy servers.
  • smoke:proxy-node verifies that HTTP requests are actually sent through the proxy using absolute-form requests, HTTPS requests use CONNECT through the proxy, NO_PROXY bypasses the proxy, and proxy credentials/path/query/fragment are redacted.
  • The smoke runner strips ambient proxy/native-proxy env from the child process so results are deterministic and do not accidentally depend on the developer or CI host environment.
  • bun test, bun run build, bun run smoke:cli, and bun run smoke:mcp verify the proxy-aware fetch wiring does not regress CLI, local MCP, auth/API service construction, or package artifacts.

Review

  • code reviewer was consulted before commit; findings were fixed, including lazy update-check proxy validation, NO_PROXY subdomain/wildcard handling, and MCP package version bump

Verification

  • bun run format:check
  • bun run typecheck
  • bun test
  • bun run build
  • bun run smoke:proxy-node
  • bun run smoke:cli
  • bun run smoke:mcp
  • bun run validate:packages

Honor HTTP_PROXY, HTTPS_PROXY, and NO_PROXY for CLI-originated traffic via a centralized proxy-aware fetch implementation. Add Node proxy smoke coverage, update the Node engine floor for undici 7, and bump @githits/mcp metadata for the engine change.
@jlitola jlitola added enhancement New feature or request dependencies Pull requests that update a dependency file labels Jul 8, 2026
@skvark

skvark commented Jul 8, 2026

Copy link
Copy Markdown
Member

Review note: createAuthCommandDependencies() and createAuthStatusDependencies() now construct createCliFetch() eagerly. Since createCliFetch() validates proxy env immediately, local-only auth paths can fail before they need network access.

Examples:

  • githits auth status with GITHITS_API_TOKEN set returns from env-token state without fetching.
  • githits logout only clears local auth storage.

With a malformed HTTP_PROXY / HTTPS_PROXY, those commands can now throw HTTP_PROXY must be an http:// or https:// proxy URL. before doing their intended local work. The update-check path already uses createLazyCliFetch() to defer this validation until the first fetch; using the lazy fetch for auth command dependencies would keep the same behavior and still validate before token refresh/login network calls.

Use lazy proxy fetch construction for auth command dependencies so local auth status/logout paths are not blocked by malformed proxy env before network use. Add regression coverage and document the dependency-construction rule for future agents.
@jlitola

jlitola commented Jul 8, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the auth dependency review note in e002450. createAuthCommandDependencies() and createAuthStatusDependencies() now inject createLazyCliFetch(), so malformed proxy env is deferred until actual network use. Added regression tests for auth command/status dependency construction under malformed HTTP_PROXY, updated AGENTS.md with the guardrail, and removed the redundant/flaky local-socket unit in favor of the deterministic unit checks plus smoke:proxy-node for real proxy behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Pull requests that update a dependency file enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants