feat: proxy 環境で全 fetch を proxy 経由にする（#54）#57
Merged
Merged
Conversation
Node 組み込み fetch は HTTP_PROXY/HTTPS_PROXY/NO_PROXY を既定で見ないため、 proxy 配下の self-hosted runner で version 解決・checksums 取得が失敗していた。 - src/proxy.ts（新規）: configureProxyFromEnv() が proxy env 検出時に undici の EnvHttpProxyAgent を global dispatcher に設定（NO_PROXY も解釈）。組み込み fetch に効く。 空文字での unset を実 proxy が後段にある場合に潰さないよう || で fallback。 - src/main.ts: run() 冒頭で呼ぶ（全ネットワーク前）。 - package.json: undici を direct dep 昇格（transitive 6.27.0 と同一）。 - self-test.yml: tinyproxy を立て、range 指定で API 解決を強制し、build-in fetch でしか到達しない api.github.com が proxy ログに出ることを検証する proxy job 追加 （tool-cache は元から proxy 対応なので asset CDN では新挙動を検証できない）。 self-test-gate の needs / 失敗検出にも proxy を追加。 Node 24 で setGlobalDispatcher が組み込み fetch に効くことをローカル smoke 確認済み。 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AECzAv2vcdATecuvLjBSa6
- README: Proxy-aware 機能 bullet を追加 - CLAUDE.md: proxy.ts の seam を追記 - TODO.md: #54 を解決済み（#57）へ移動 + #55/#56 を backlog に追加 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AECzAv2vcdATecuvLjBSa6
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (9)
✅ Files skipped from review due to trivial changes (3)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughSummary by CodeRabbit
Walkthroughプロキシ環境変数を読む ChangesProxy-aware fetch bootstrap
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 markdownlint-cli2 (0.22.1)CLAUDE.mdmarkdownlint-cli2 wrapper config was not available before execution TODO.mdmarkdownlint-cli2 wrapper config was not available before execution Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #57 +/- ##
==========================================
+ Coverage 69.91% 70.32% +0.40%
==========================================
Files 11 13 +2
Lines 236 246 +10
Branches 61 62 +1
==========================================
+ Hits 165 173 +8
- Misses 62 64 +2
Partials 9 9
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CLAUDE.md`:
- Line 54: The Host allowlist and preflight documentation in CLAUDE.md is now
inconsistent with the new proxy behavior. Update the `download.ts` preflight
description so it reflects that, after `configureProxyFromEnv()` runs in
`run()`, built-in `fetch` can go through the proxy instead of saying it ignores
runner proxy settings; if needed, adjust the `Host allowlist` wording to match
the new `proxy.ts` note and keep the two entries aligned.
In `@src/proxy.ts`:
- Line 24: Remove the undici-based proxy setup from proxy.ts: the top-level
setGlobalDispatcher(new EnvHttpProxyAgent()) call should be eliminated, along
with the EnvHttpProxyAgent dependency. In run(), initialize Node 24’s native
env-proxy support by setting NODE_USE_ENV_PROXY=true before any fetch usage (or
by applying the equivalent --use-env-proxy runtime flag), so proxy behavior
comes from the built-in fetch environment handling instead of a custom global
dispatcher.
In `@TODO.md`:
- Around line 56-57: The TODO table rows for the `#55` and `#56` entries are
missing the empty `備考` cell, so the column count no longer matches the 5-column
header. Update the Markdown table in `TODO.md` by adding a blank final cell to
each of those rows so they align with the existing table structure and stay
consistent with the other entries.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: eecc7c8f-fb12-4864-bb49-636e76cf4352
⛔ Files ignored due to path filters (3)
.github/workflows/self-test.ymlis excluded by!**/*.ymlpackage.jsonis excluded by!**/*.jsonpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!**/*.yaml
📒 Files selected for processing (5)
CLAUDE.mdREADME.mdTODO.mdsrc/main.tssrc/proxy.ts
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 8 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
- proxy.ts: 不正 proxy URL（EnvHttpProxyAgent constructor throw）を actionable な Error に変換（cause 付与で preserve-caught-error も満たす） - download.ts: proxy 対応後に不正確になった stale コメントを修正 - self-test.yml: proxy job に check-latest:true。range が tool-cache に hit すると API 解決がスキップされ、api.github.com アサートが空振りするため - tests/proxy.test.ts: gating / 小文字変数 / || fallback / 不正URL の unit テスト Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AECzAv2vcdATecuvLjBSa6
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/proxy.test.ts`:
- Around line 3-16: `vi.mock('undici', ...)` is hoisted, so the test-scoped
spies used by `setGlobalDispatcher` and `EnvHttpProxyAgent` must be initialized
before mock evaluation to avoid TDZ `ReferenceError`s. Move the spy creation
into `vi.hoisted()` and keep using those hoisted references inside the `undici`
mock in `tests/proxy.test.ts`. Make sure the mocked `setGlobalDispatcher` and
constructor spy still behave the same for `configureProxyFromEnv`.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3067ac68-f4b0-4ec0-ade9-1857338831f6
⛔ Files ignored due to path filters (1)
.github/workflows/self-test.ymlis excluded by!**/*.yml
📒 Files selected for processing (3)
src/download.tssrc/proxy.tstests/proxy.test.ts
✅ Files skipped from review due to trivial changes (1)
- src/download.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/proxy.ts
There was a problem hiding this comment.
3 issues found across 4 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
undici 6.x の setGlobalDispatcher が Node 24 組み込み fetch に効くかは symbol 共有のバージョン依存で脆弱（CodeRabbit Major 指摘）。組み込み fetch ではなく npm undici の fetch を直接使えば、同一 undici インスタンス上で dispatcher が確実に適用される。 - src/fetch.ts（新規）: undici の fetch を string-URL シグネチャで re-export - src/github.ts: ./fetch から import（組み込み fetch をやめる） - tests: globalThis.fetch の stub をやめ ../src/fetch を mock - CLAUDE.md: fetch shim / proxy の seam を追記 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AECzAv2vcdATecuvLjBSa6
- self-test.yml: proxy job が apt 自動起動サービスと :8888 衝突しログを /tmp に残せず assert がコケていた。サービス停止 + User/Group/PidFile を runner 側に明示 + ログ生成確認を追加（cubic/CI 失敗の修正） - proxy.ts: 不正 proxy URL のエラーで値を出さず変数名のみ報告（URL に embedded credentials がログ漏洩するのを防止、cubic P2） - proxy.test.ts: vi.hoisted で spy を宣言（vi.mock hoisting、CodeRabbit Critical）+ resetAllMocks で mockImplementation の漏れ防止（cubic P2） - TODO.md: backlog 表の #55/#56 に空 備考セル（MD056、CodeRabbit） Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AECzAv2vcdATecuvLjBSa6
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
概要
Node 組み込み
fetchはHTTP_PROXY/HTTPS_PROXY/NO_PROXYを既定で参照しないため、proxy 配下の self-hosted runner では version 解決・checksums 取得が失敗していた（#42 のレビューで判明した既存制約）。本 PR で全fetchを proxy 経由にする。設計判断（公式確認 + ローカル smoke 済み）
setGlobalDispatcher(new EnvHttpProxyAgent())が正準解。起動時に1回設定で組み込みfetch全部が proxy 経由になり、secureFetchも既存テスト（global fetch を stub）も無変更setGlobalDispatcherが組み込み fetch に効くことをローカル smoke 確認（存在しない proxy → fetch が ECONNREFUSED）NODE_USE_ENV_PROXYは不採用（前者は全書き換え、後者はプロセス起動フラグでコードから使えない）変更内容
src/proxy.ts（新規）:configureProxyFromEnv()— proxy env 検出時のみEnvHttpProxyAgentを global dispatcher に設定（NO_PROXYも解釈、未設定なら no-op）。空文字 unset を後段の実 proxy が潰さないよう||で fallbacksrc/main.ts:run()冒頭（全ネットワーク前）で呼ぶpackage.json:undiciを direct dep へ昇格（transitive 6.27.0 と同一、install 増なし）.github/workflows/self-test.yml: proxy E2E job を追加。tinyproxy を立て、range 指定で API 解決を強制し、built-in fetch でしか到達しないapi.github.comが proxy ログに出ることを検証（tool-cache は元から proxy 対応なので asset CDN では新挙動を検証できない）。gate の needs / 失敗・キャンセル検出にもproxy追加検証
pnpm run all✅（typecheck / lint / 69 tests / build）/actionlint✅ /typos✅??→||・proxy アサート強化の2点を反映済み参照
🤖 Generated with Claude Code
Summary by cubic
Route all action fetches through the runner’s HTTP(S) proxy when
HTTP_PROXY/HTTPS_PROXYare set, honoringNO_PROXY. Uses anundici-based fetch shim so proxying is reliable across Node versions, fixing version resolution and checksum fetches on proxied self-hosted runners.New Features
configureProxyFromEnv()installsundici’sEnvHttpProxyAgentviasetGlobalDispatcher; respectsHTTP_PROXY/HTTPS_PROXY/NO_PROXY(upper/lowercase), no-op without a proxy, uses||so empty vars don’t mask real values, and fails fast on malformed URLs while naming only the env var (no credential leak).undici-backed shim (src/fetch.ts) so the dispatcher is always honored;github.tsnow imports from it; tests mock./fetchinstead ofglobalThis.fetch./tmp, force API resolution withcheck-latest: true, and assertapi.github.comis proxied.Dependencies
undicias a direct dependency.Written for commit c3faba4. Summary will update on new commits.