Skip to content

fix: rebase patches onto upstream 1.136.0 - #60

Merged
sbs44 merged 2 commits into
mainfrom
fix/upstream-1.136.0-build
Sep 3, 2026
Merged

fix: rebase patches onto upstream 1.136.0#60
sbs44 merged 2 commits into
mainfrom
fix/upstream-1.136.0-build

Conversation

@claude

@claudeclaudeBot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Fixes the nightly build failure on vscode 1.136.0.

Refs #59

Root causes

Two distinct failures, the second only reachable once the first was fixed.

1. Four patches stopped applying

The build log named only 00-brand-remove-branding.patch, because
prepare_vscode.sh halts on the first rejection.
./dev/dry-apply-patches.sh found four broken patches:

PatchUpstream change that broke it
00-brand-remove-brandingPromptsType.skill diagnostic reworded "VS Code agents" -> "VS Code skills"
00-vsce-use-custom-libbrace-expansion in the vsce dep tree 5.0.7 -> 5.0.9
21-policy-use-custom-lib@vscode/os-proxy-resolver ^0.3.0 -> ^0.4.0
51-ext-copilot-remove-itnew @devcontainers/cli dep split the anchored context; agentService.registerProvider renamed to providerService.registerProvider; ten copilot files changed contents

The first three are context-line drift, fixed in place so the !!TOKEN!!
placeholders are untouched. 51 was regenerated by applying it raw and
re-diffing, so its 21 !!APP_NAME!! tokens round-trip.

51's deletion hunks are the notable part: for ten files upstream changed
the contents, so git apply reported removal patch leaves file contents
and emitted no.rej — the deletions would have silently vanished from
a naive regeneration. Post-regeneration checks: deleted file mode count
unchanged at 24, deletion set byte-identical to the old patch, no files
dropped, tokens preserved, no literal BradfordCode leaked in.

1.136.0 also added two new importers of the removed @github/copilot-sdk
(copilotSecondaryAssignmentContext.ts and its test). Both get the same
local type alias the patch already applies to GitHubTelemetryNotification
elsewhere.

upstream/stable.json is bumped to 1.136.0 / 520fb30b2d3d324b4cb2342f6e88e2cd93751de1
(resolved via git ls-remote refs/tags/1.136.0, not the update API) so the
PR build targets the tree these patches were rebased onto.

2. npm ci crashed resolving the electron version

With patching fixed, the build reached prepare_vscode.sh's npm ci and died:

TypeError: Cannot read properties of null (reading '1')
at getElectronVersion (build/lib/electronVersion.ts:13:57)
at ensureElectronTypes (build/npm/electronTypes.ts:29:30)
at main (build/npm/postinstall.ts:242:8)

1.136.0 added ensureElectronTypes() to postinstall.ts; it is absent from
1.135.0's, which is why this never fired before. It calls
getElectronVersion(), which non-null-asserts a /^target="(.*)"$/ match
against .npmrc.

prepare_vscode.sh swaps vscode's .npmrc for this repo's npmrc for the
duration of npm ci (mv .npmrc .npmrc.bak; cp ../npmrc .npmrc) and restores
it after. Our npmrc deliberately omits the electron settings, so during that
window there is no target= line and the assertion dereferences null.

00-build-fix-electron-version-lookup.patch reads the version from
.npmrc.bak when the live .npmrc has no target=. Copying target= into
our npmrc instead would be wrong: without the accompanying
runtime="electron" and disturl, node-gyp would read it as a Node.js target
and try to fetch headers for a nonexistent v42.10.0. The other
getElectronVersion() callers all run after .npmrc is restored, so they keep
taking the original path.

Verification

./dev/dry-apply-patches.sh — all patches apply cleanly against
520fb30b2d3d324b4cb2342f6e88e2cd93751de1.

CI_BUILD=yes ./dev/ci-verify.sh --commit 520fb30b2d3d324b4cb2342f6e88e2cd93751de1
...
[compile-client] Finished compile-src /vscode/src/tsconfig.json with 0 errors.
ci-verify: PASS (compile, tag 1.136.0)

Tier reached: compile passed (overlay + patches + npm ci + TypeScript
compile). Not run: --full (vscode-min-prepack minify + ASCII hygiene) and
packaging/signing, so those remain covered only by the PR build.

The nightly build failed applying 00-brand-remove-branding.patch against
vscode 1.136.0. prepare_vscode.sh halts on the first rejection, so the log
named only that one; ./dev/dry-apply-patches.sh found four broken patches.
Rebased against 520fb30b2d3d324b4cb2342f6e88e2cd93751de1:
- 00-brand-remove-branding: upstream reworded the PromptsType.skill
diagnostic from "VS Code agents" to "VS Code skills".
- 00-vsce-use-custom-lib: brace-expansion in the vsce dep tree went
5.0.7 -> 5.0.9.
- 21-policy-use-custom-lib: @vscode/os-proxy-resolver went ^0.3.0 -> ^0.4.0.
- 51-ext-copilot-remove-it: regenerated. Upstream added @devcontainers/cli
between the anchored context lines, renamed agentService.registerProvider
to providerService.registerProvider, and changed the contents of ten
copilot files whose deletion hunks then silently no-op'd ("removal patch
leaves file contents" emits no .rej). Deletion count is unchanged at 24
and the !!APP_NAME!! tokens are preserved.
1.136.0 also added two new importers of the removed @github/copilot-sdk
(copilotSecondaryAssignmentContext.ts and its test); both get the same
local type alias the patch already uses for GitHubTelemetryNotification.
Also bumps upstream/stable.json to 1.136.0 so the PR build targets the tree
these patches were rebased onto.
With the patches rebased, the build got past patching and failed in
prepare_vscode.sh's npm ci:
TypeError: Cannot read properties of null (reading '1')
at getElectronVersion (build/lib/electronVersion.ts:13:57)
at ensureElectronTypes (build/npm/electronTypes.ts:29:30)
at main (build/npm/postinstall.ts:242:8)
1.136.0 added ensureElectronTypes() to postinstall; it is absent in
1.135.0's postinstall.ts, which is why this never fired before. It calls
getElectronVersion(), which non-null-asserts a /^target="(.*)"$/ match
against .npmrc.
prepare_vscode.sh swaps vscode's .npmrc for this repo's npmrc for the
duration of npm ci (mv .npmrc .npmrc.bak; cp ../npmrc .npmrc) and restores
it afterwards. Our npmrc deliberately omits the electron settings, so
during that window there is no target= line and the assertion dereferences
null.
Read the version from .npmrc.bak when the live .npmrc has no target=.
Copying target= into our npmrc instead would be wrong: without the
accompanying runtime="electron" and disturl, node-gyp would read it as a
Node.js target and try to fetch headers for a nonexistent v42.10.0. The
other getElectronVersion() callers all run after .npmrc is restored, so
they keep taking the original path.
@github-actions

Copy link
Copy Markdown
Contributor

Build green at 576a609 (run). Ready for review — assigned to @sbs44.

@sbs44
sbs44 merged commit 42e1e60 into mainSep 3, 2026
8 checks passed
@sbs44
sbs44 deleted the fix/upstream-1.136.0-build branch September 3, 2026 11:47
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@sbs44