fix(build): proxy-aware version resolution and Docker builds, hardened - #408
Merged
Conversation
- version resolution: replace npm view with direct registry.npmjs.org curl; npm view honored .npmrc registry (e.g. npmmirror) which could return stale metadata for recently-republished packages - Docker build: forward HTTP_PROXY/HTTPS_PROXY from host env to all docker build/buildx commands via BuildKit predefined ARGs - proxy URL rewrite: 127.0.0.1/localhost -> host.docker.internal so build containers reach the host proxy instead of own loopback - verbose proxy logging in install-agent-tooling.sh and version-upgrade.sh Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Review fixes for the re-landed proxy work (#407, split from #404): - docker build gets --add-host host.docker.internal:host-gateway whenever proxy args are forwarded: the localhost rewrite was dead on native Linux Engine (Docker Desktop/OrbStack resolve the name implicitly, Engine does not). Verified against a real daemon. - localhost rewrite now catches authenticated proxies: the old patterns anchored on ://127.0.0.1 and missed http://user:pass@127.0.0.1:7890 (confirmed via make -n). Make macro and version-upgrade sed both handle @127.0.0.1/@localhost. - proxy userinfo never printed raw: log_proxy_config (build logs) and version-upgrade's forwarding display redact user:pass@ to ***@. Residual: make's own recipe echo still expands proxy values in a local terminal; not worth silencing every build recipe over. - npm registry fetch restores npm view's soft-fail contract: transient curl failure returns empty and hits load_versions' warn-and-fallback branch instead of aborting under set -euo pipefail (that branch was dead code for npm tools). - tests/version-upgrade.sh: fake curl now serves realistic dist-tags + packument fixtures instead of empty strings (the stale npm view mocks made the suite pass while no builds ran, then die on the missing docker-build.log — the #404 CI red). npm mock is a tripwire: any npm call fails loudly. New scenarios: proxied build asserts rewrite + host-gateway + no credential leak; registry outage asserts warnings, fallback to current, exit 0, no builds. Close #407 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Updates version resolution and Docker build behavior to be proxy-aware and more robust by switching npm version lookups to direct npm registry HTTP calls, forwarding proxy env vars into builds (with localhost rewrite), and expanding test coverage for proxy and outage scenarios.
Changes:
- Replace
npm view-based version lookups withcurltoregistry.npmjs.org(dist-tags + packument time). - Forward HTTP(S) proxy env vars into Docker builds and rewrite
127.0.0.1/localhosttohost.docker.internal, adding host-gateway mapping when proxy forwarding is active. - Harden
tests/version-upgrade.shto use realistic curl fixtures, add proxy-forwarding assertions (including redaction), and validate “registry outage” soft-fail behavior.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/version-upgrade.sh | Reworks mocks/fixtures for registry-based resolution; adds proxy + outage scenarios. |
| scripts/version-upgrade.sh | Adds proxy forwarding + redacted proxy logging and injects proxy args into docker builds. |
| scripts/update-version-pins.sh | Switches npm version pin fetching from npm view to registry HTTP dist-tags. |
| scripts/release-utils.sh | Implements registry-based npm latest/date resolution with soft-fail behavior. |
| scripts/install-agent-tooling.sh | Adds proxy config logging with userinfo redaction for build logs. |
| Makefile | Forwards proxy args into docker build/buildx and adds host-gateway mapping when needed. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| [[ -n ${no_proxy:-} ]] && PROXY_ARGS+=(--build-arg "no_proxy=$no_proxy") | ||
| # host.docker.internal resolves implicitly on Docker Desktop/OrbStack only; | ||
| # native Linux Engine needs the explicit host-gateway mapping during build. | ||
| [[ ${#PROXY_ARGS[@]} -gt 0 ]] && PROXY_ARGS+=(--add-host "host.docker.internal:host-gateway") |
Comment on lines
+223
to
+225
| for _pa in "${PROXY_ARGS[@]}"; do | ||
| echo -e " ${DIM}$(_redact_proxy "${_pa#--build-arg }")${RESET}" | ||
| done |
![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: f554d24423
ℹ️ 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".
Comment on lines
+55
to
+58
| $(if $(HTTP_PROXY),--build-arg HTTP_PROXY=$(call _dproxy,$(HTTP_PROXY))) \ | ||
| $(if $(HTTPS_PROXY),--build-arg HTTPS_PROXY=$(call _dproxy,$(HTTPS_PROXY))) \ | ||
| $(if $(http_proxy),--build-arg http_proxy=$(call _dproxy,$(http_proxy))) \ | ||
| $(if $(https_proxy),--build-arg https_proxy=$(call _dproxy,$(https_proxy))) \ |
There was a problem hiding this comment.
When a proxied make build uses authenticated proxy URLs containing Make or shell metacharacters, these unquoted expansions corrupt the value before Docker receives it; for example, HTTP_PROXY='http://user:pa$ss@127.0.0.1:7890' make -n build-core renders HTTP_PROXY=http://user:pas@host.docker.internal:7890, and &/; in a password would be interpreted by the shell. This breaks the proxy-aware build path specifically for the authenticated proxies this change is trying to support; the proxy values need to be escaped/quoted before being placed into recipe commands.
Useful? React with 👍 / 👎.
lroolle
added a commit
that referenced
this pull request
Jul 10, 2026
The product ships four agents; half the storefront said three. Fix the surfaces so what people (and their agents) read matches what actually runs: - kill three-agent drift: docs.deva.sh front page, mkdocs site_description, deva.sh --help header — all now know Grok exists - llms.txt at repo root: identity, the four agents, the no-theater auth model, install, curated docs entry points — an agent can recommend and install deva from one fetch. README grew a one-line pointer for non-human readers. - nightly honesty: 2026-07-09 nightly died on a transient ports.ubuntu.com mirror sync and partially published (base pushed, rust didn't — the two nightly tags diverged a day). apt now retries (Acquire::Retries 3, both Dockerfiles) and a verify job asserts all four published manifests actually resolve. Assert the artifact, not the exit code. - CHANGELOG: backfill the #407 proxy entry that #408 forgot, add this Close #409 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Merged
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
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.
Re-lands the proxy work split out of #404 (b6360a6), plus the fixes for every gap the review round found there.
Close #407
What
Original commit, unchanged in intent:
npm viewwith direct registry.npmjs.org curl (npm viewhonors.npmrcregistry overrides, e.g. npmmirror, which can serve stale metadata for recently-republished packages)Hardening on top:
--add-host host.docker.internal:host-gatewayondocker buildwhenever proxy args are forwarded — the rewrite was dead on native Linux Engine (Desktop/OrbStack resolve the name implicitly, Engine does not). Verified against a real daemon: flag accepted, name resolves in-build.://127.0.0.1and missedhttp://user:pass@127.0.0.1:7890(confirmed viamake -n). Both the Make macro and the version-upgrade sed handle@127.0.0.1/@localhostnow.log_proxy_config(lands in docker build logs) and version-upgrade's forwarding display redactuser:pass@to***@. Residual, deliberate: make's own recipe echo still expands proxy values in your local terminal — silencing every build recipe isn't worth it, and CI never carries a localhost proxy.npm view's soft-fail contract: transient curl failure returns empty and reachesload_versions' warn-and-fallback branch instead of aborting underset -euo pipefail(Copilot's valid half on feat: add grok agent (official xAI Grok CLI) #404; the URL-encoding half was tested false — both raw and %2f scoped paths return 200)..gitignorehunk from b6360a6.Root cause of the #404 CI red, now covered
The old test's fake curl printed an empty string for every request while stale
npm viewfixtures sat unused: every lookup "failed", the script fell back to image-current versions, decided nothing needed upgrading, exited before any build, and the test died reading the missing docker-build.log.Tests
🤖 Generated with Claude Code