Repository navigation
fix(docker): make optional native dependency fallback explicit - #1605
Merged
Cedric Vidal (cedricvidal) merged 5 commits intoNov 5, 2026
Merged
Conversation
Test Results (Node.js 22)test: Run #446
🎉 All tests passed! |
Test Results (Node.js 22)test: Run #449
🎉 All tests passed! |
Test Results (Node.js 22)test: Run #455
🎉 All tests passed! |
Cedric Vidal (cedricvidal)
requested a review
from Wassim Chegham (manekinekko)
November 4, 2026 01:57
Test Results (Node.js 22)test: Run #457
🎉 All tests passed! |
Cedric Vidal (cedricvidal)
changed the base branch from
cedricvidal-verify-stack-endpoints
to
cedricvidal-decouple-pnpm-bootstrap
November 4, 2026 02:49
Test Results (Node.js 22)test: Run #461
🎉 All tests passed! |
Wassim Chegham (manekinekko)
approved these changes
Nov 4, 2026
Wassim Chegham (manekinekko)
left a comment
Member
There was a problem hiding this comment.
LGTM. The optional native-build fallback is narrowly scoped and preserves the required dependency build scripts. One non-blocking test-hardening observation inline.
Test Results (Node.js 22)test: Run #479
🎉 All tests passed! |
Cedric Vidal (cedricvidal)
marked this pull request as ready for review
November 5, 2026 00:37
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Cedric Vidal (cedricvidal)
force-pushed
the
cedricvidal-fix-optional-native-fallback
branch
from
November 5, 2026 00:40
51b45e2 to
c3ce43c
Compare
Test Results (Node.js 22)test: Run #499
🎉 All tests passed! |
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.
Summary
Clean dependency installs in minimal Node images attempted two optional native builds from the transitive
ssh2dependency chain:ssh2's bundled crypto binding andcpu-features. When Python, a compiler, or node-gyp prerequisites were unavailable, those scripts printed alarming error-shaped diagnostics before correctly continuing withssh2's supported JavaScript implementation. The install succeeded, but the output looked like a genuine dependency failure.This change makes that fallback deliberate and quiet by moving only
ssh2andcpu-featuresfrom pnpm'sonlyBuiltDependenciesallowlist toignoredBuiltDependencies. This is safe for Scope becausessh2is reached only through Docker test tooling, its package documents the native addons as optional optimizations, and Dockerode's local-socket path works through the JavaScript implementation. Required dependency scripts remain explicitly allowlisted, includingesbuild,keytar, andprotobufjs. No install command uses--ignore-scripts,ignoreDepScripts, stderr suppression, error filtering, or a success-shaped wrapper around a failing install.Regression assertions verify the narrow ignored list, preserve required build allowlisting, and reject broad lifecycle-script suppression.
CONTRIBUTING.mddocuments the explicit fallback policy.This is layer 4 of the six-PR native stack #1610 and depends directly on #1603. The branch has been synchronized with the current lower-layer base using normal commits; this PR is based on
cedricvidal-decouple-pnpm-bootstrapand does not include the later Portal Git metadata or storage-guidance layers.Demos
N/A — dependency installation behavior only; validation is captured in build and test output.
Before
N/A — clean minimal Node image installs emitted node-gyp/Python/compiler failures from optional
ssh2andcpu-featuresnative builds, then continued with the valid JavaScript fallback.After
N/A — a clean dependency-layer install deliberately omits those two optional native scripts, emits no native compiler failure output, still runs required lifecycle scripts, and loads the Dockerode JavaScript path successfully.
Testing
pnpm exec vitest run scripts/bootstrap-pnpm.test.ts --reporter=default: 1 file, 8 tests. The suite assertsssh2/cpu-featuresare narrowly ignored, required scripts remain allowlisted, and Docker install commands do not use--ignore-scripts.pnpm --filter shared build && pnpm --filter telemetry build && pnpm --filter test-utils build && pnpm --filter coder-acp-copilot build && pnpm --filter coder-acp-claude-code build.pnpm --filter test-utils exec node --input-type=module -e "...": Dockerode local-socket client initialized through the JavaScript path./validationworkspace andpnpm install --offline --force --frozen-lockfile. The requiredesbuildpostinstall ran successfully;ssh2andcpu-featuresproduced no install-script output.node-gyp,gyp ERR,Failed to build optional crypto binding, missing Python/compiler, orcpu-featuresbuild output.ssh2andcpu-featurespackage trees and confirmed they contain no native.nodebindings.pnpm headers:check: all 967 in-scope files have the required header after final-base synchronization.git diff --check.66dd8c65e09a87b8dfb2a09d626dd091e5d192ae: CI Summary, Windows Worker Build, Unit Tests (22; 3,257/3,257), Build Applications, Lint Code, CLI Bundle Integration Tests, Third-party NOTICE, License Headers, detect-changes, gitleaks, CLA, and changed-file labeling. Integration, LLM evaluation, and gateway jobs were skipped by their normal change-detection conditions.Environment limitation: the first fully fresh
node:24-slimvalidation reached the unchanged pinned pnpm bootstrap, but this Docker runtime could not establish TLS toregistry.npmjs.org(ECONNRESET) through either Corepack or npm. The successful no-cache dependency-layer validation therefore reused the local pnpm content-addressable store in offline mode while creating a fresh workspace andnode_modules. No credentials were used or captured, and the layer-3 registry/bootstrap contract was not changed.Documentation and compatibility
Updated
CONTRIBUTING.mdto document the explicit pure-JavaScript fallback and the requirement to keep unrelated dependency build scripts enabled. No dependency versions or lockfile entries changed, so NOTICE regeneration was not required. There are no breaking changes, runtime API changes, Portal/CLI behavior changes, database migrations, or upgrade steps.Checklist
up()/down()and keep it CosmosDB-compatible. N/A — no database changes.NOTICE/NOTICE-REVIEW.txtwithpnpm noticeas needed. N/A — dependency versions and lockfile are unchanged; Third-party NOTICE passed in final-base CI.