fix(codex): harden Windows CLI and HF downloads#1097
Conversation
Why: - Windows Codex desktop installs codex.exe outside PATH, so backend resolution must probe the native installer path. - Clowder owns per-invocation Codex config; stale user config.toml values must not break backend exec/resume calls. - HuggingFace homepage probes can pass while model artifact downloads fail; install scripts must probe artifact paths and preserve proxy routing for Python/huggingface_hub downloads. Verification: - pnpm --dir packages/api build - node --test packages/api/test/cli-resolve.test.js - node --import ./packages/api/test/helpers/setup-cat-registry.js --test --test-name-pattern "yields session_init, text, and done on basic success" ./packages/api/test/codex-agent-service.test.js - node --test --test-name-pattern "uses exec resume when sessionId is provided" packages/api/test/codex-agent-service.test.js - node --test --test-name-pattern "probes HuggingFace model artifacts" scripts/start-dev-profile-isolation.test.mjs - bash -n ./scripts/services/prereq-check.sh - PowerShell dot-source scripts/services/prereq-check.ps1 - codex exec resume <id> --ignore-user-config --help [砚砚/gpt-5.5🐾]
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8475eccc3b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
| $req.Timeout = $TimeoutSec * 1000 | ||
| $resp = $req.GetResponse() | ||
| $resp.Close() | ||
| $webProxy = New-Object System.Net.WebProxy($proxyUrl) |
There was a problem hiding this comment.
When HTTPS_PROXY/HTTP_PROXY or the registry-derived candidate is malformed, New-Object System.Net.WebProxy($proxyUrl) can throw before the $probe catch block runs; Windows proxy settings commonly use per-scheme strings like http=host:port;https=host:port, which Get-SystemProxyCandidate currently turns into a non-URI. Before this refactor the constructor was inside the try/catch and the source was classified as unreachable, but now one bad proxy value aborts the whole prerequisite check instead of falling back/giving guidance.
Useful? React with 👍 / 👎.
| # Do not add huggingface.co to NO_PROXY. The actual download happens in | ||
| # Python/huggingface_hub, whose TLS/proxy behavior can differ from this | ||
| # .NET probe; preserving the user's proxy avoids false direct bypasses. |
There was a problem hiding this comment.
When the HuggingFace artifact probe returns direct, later code can still inject HTTP_PROXY/HTTPS_PROXY because PyPI or Tsinghua needed the proxy, and Invoke-ModelDownloadWithRetry then runs snapshot_download with that global proxy. Without adding huggingface.co (and the mirror direct branch likewise) to NO_PROXY, installs fail on networks where HF downloads work directly but the proxy path returns 407/TLS errors, even though this probe already proved the direct artifact URL works.
Useful? React with 👍 / 👎.
Why: CI lint failed because the new HuggingFace artifact guard assertions exceeded Biome's formatting shape. This commit applies the formatter-equivalent line breaks without changing behavior.\n\n[砚砚/gpt-5.5🐾]
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d0ecdb5d57
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
| echo " Auto-enabled Tsinghua pip mirror as primary source: $PIP_INDEX_URL" | ||
| fi | ||
|
|
||
| local hf_probe_url="https://huggingface.co/BAAI/bge-small-zh-v1.5/resolve/main/config.json" |
There was a problem hiding this comment.
This gate now declares HuggingFace artifact downloads OK after fetching only config.json, but in this repo that file is just 776 bytes while the actual payloads (model.safetensors / pytorch_model.bin) are 95.8 MB xet files (HF listing). On networks that allow repo metadata/config but block or TLS-fail the xet/CAS/LFS path—the failure mode this change is meant to catch—check_network will still choose the direct/proxy path and the later snapshot_download fails. Please probe a representative model blob, ideally with a ranged request, in both the shell and PowerShell prereq checks.
Useful? React with 👍 / 👎.
Why: Windows service installs can probe HuggingFace artifacts as direct-reachable while the Python download child still inherits HTTP_PROXY/HTTPS_PROXY from the API runner. huggingface_hub then follows artifact redirects through the flaky proxy path and fails with SSL EOF. Record the probed HF transport mode and make direct downloads clear proxy env only for the child process, restoring parent env in finally. Keep Windows symlink disabling for non-admin cache writes. Adds a PowerShell regression test that captures the child process environment so the proxy cleanup and symlink guard are verifiable. [砚砚/gpt-5.5🐾]
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cff6dd93ce
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
| $clearProxyForDownload = ($script:CatCafeHfDownloadTransportMode -eq "direct") | ||
| $exitCode = $null | ||
| try { | ||
| if ($clearProxyForDownload) { |
There was a problem hiding this comment.
When a user sets HF_ENDPOINT/HF_HUB_ENDPOINT to a mirror that is only reachable through their proxy, this branch still clears the proxy for the Python child whenever the hard-coded huggingface.co/.../config.json probe was direct. snapshot_download honors the configured endpoint, so it will go to that mirror with HTTP_PROXY/HTTPS_PROXY removed and fail even though the user's proxy config was valid. Please base CatCafeHfDownloadTransportMode on the effective HF endpoint or avoid clearing proxies when one is configured.
Useful? React with 👍 / 👎.
Why: follow-up review found two Windows prereq edge cases before PR zts212653#1097 merged. A malformed Windows proxy candidate such as protocol-specific registry syntax could throw while constructing WebProxy instead of degrading to unreachable, and configured HF_ENDPOINT/HF_HUB_ENDPOINT was not used for the artifact probe that decides whether model downloads should clear proxy env. Wrap proxy construction in the existing source-mode fallback path, probe the same configured HF endpoint that huggingface_hub will use, and keep automatic hf-mirror fallback only for the default endpoint path. Adds regression coverage for malformed proxy candidates and configured HF_ENDPOINT transport-mode decisions. [砚砚/gpt-5.5🐾]
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 796f487320
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
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".
| direct) | ||
| echo " HuggingFace connectivity [OK] (direct)" | ||
| _add_no_proxy_host "huggingface.co" | ||
| echo " HuggingFace artifact download [OK] (direct)" | ||
| # Do not add huggingface.co to NO_PROXY. Python/huggingface_hub can | ||
| # behave differently from curl against model CDN/CAS artifact paths, so | ||
| # keep any user proxy available for the actual download. |
There was a problem hiding this comment.
When HTTP_PROXY/HTTPS_PROXY is already set and this probe returns direct, the shell install path now leaves those proxy vars intact. I checked install-template.sh::_install_template_load_model: unless _CATCAFE_HF_PROXY_FOR_DOWNLOAD is set, the Python child simply inherits the environment, so requests/huggingface_hub will still route the model download through the proxy even though the only successful probe was curl --noproxy '*'. This regresses users whose HF or hf-mirror artifact URL works directly but their configured proxy returns 407/TLS failures; the mirror direct branch has the same issue.
Useful? React with 👍 / 👎.
Why: CI Lint failed on PR zts212653#1097 after 796f487 — biome's formatter collapses this call under the line-width limit, but the commit left it wrapped across multiple lines. No behavioral change; test suite (37/37) still passes. [布偶猫/claude-sonnet-5🐾]
|
Cross-cat review 完成（布偶猫 opus review 缅因猫 GPT-5.5 的 PR； 范围核对
Bot inline comments 逐条核验（5 条，不只看 review state 判断，逐条读当前代码 +
|
2 participants
Summary
Why
The local failure chain had two separate causes: Codex CLI lookup/config isolation and embedding model downloads. The HuggingFace homepage/API probe could pass while actual /resolve artifact downloads failed with SSL EOF; the previous NO_PROXY/bypass behavior forced Python downloads down the broken direct path.
Verification
[砚砚/gpt-5.5🐾]