build(xterm): make the regenerator runnable on Windows and drop dead paths

Readiness review on the restore found one blocking gap and two cheap
cleanups. None of them change the emitted patch, which is byte-identical
before and after.

The generator could not run on Windows at all. Three sites called npm
through execFileSync with shell:false, but npm ships as npm.cmd there,
execFile applies no PATHEXT, and since CVE-2024-27980 it refuses a .cmd
target without a shell. That matters because this harness arms a
blocking gate whose documented remedy is --write, so a Windows
contributor who tripped the gate had no remedy except hand-editing a 7MB
minified bundle, which is the practice the gate exists to abolish. Four
sibling scripts in config/scripts already handle this; the fix follows
them and lands in run(), so the manifest-driven build step is covered
too. git and tar are real executables in System32 and keep resolving
without a shell, which avoids quoting exposure on paths with spaces.

deleteGeneratedSourcemaps was unreachable, since the policy is include.
Deleting it left "delete" as a legal policy value that nothing honoured,
so a manifest asking for it would have silently shipped sourcemaps that
do not match the bundle. The enum is narrowed and an unrecognised policy
now throws rather than falling through.

generatedHunks moved into the test file rather than being dropped; its
partition assertion, that generated and source hunks reconstruct the
whole patch, is worth keeping.

The -text attribute now covers all five patch files. pnpm hashes each of
them byte-for-byte, so the CRLF hazard the xterm patch was protected
from applies equally to node-pty and the three addons. All five were
already LF in the object DB, so this pins existing behaviour. -diff
stays scoped to the xterm patch, since the others are readable.

The doc's claim that the addons reproduce byte for byte is now dated and
marked a one-off measurement rather than an invariant, because nothing
re-runs it.

Effective lines fall from 591 to 568 against the 600 budget. Still the
largest file in config/scripts, and adding a second package to the
manifest would need a split first.

.gitattributes

@@ -12,8 +12,9 @@
 /src/cli/bundled-skill-guides.ts text eol=lf
 # Bundled plugin trees are byte-hashed; CRLF checkout would break the pinned hash.
 /resources/plugins/** text eol=lf
-# pnpm hashes this generated patch byte-for-byte, so a CRLF checkout breaks the install;
-# its minified bundle lines also make a diff nobody can read. Review the hand-written
-# source patch under xterm-src/ instead.
-/config/patches/@xterm__xterm@*.patch -diff -text
+# pnpm hashes every patch byte-for-byte, so a CRLF checkout breaks the install.
+/config/patches/*.patch -text
+# The xterm bundle hunks also make a diff nobody can read; review the hand-written
+# source patch under xterm-src/ instead. The sibling patches stay diffable.
+/config/patches/@xterm__xterm@*.patch -diff
 /config/patches/xterm-src/*.patch text eol=lf

config/scripts/regenerate-xterm-patches.mjs

@@ -139,13 +139,6 @@ export function assertSourceDerivationsAgree(checkoutSource, patchText) {
   )
 }
 
-/** The derived half of a patch: build output, never edited by hand. */
-export function generatedHunks(patchText, generatedPaths) {
-  return selectPatchEntries(patchText, (file) =>
-    generatedPaths.some((prefix) => file.startsWith(prefix))
-  )
-}
-
 export function stampVersionSource(source, version) {
   const stamped = source.replace(
     /export const XTERM_VERSION = '[^']+';/,
@@ -196,9 +189,11 @@ export function assertBuildStepsAllowed(manifest) {
   }
 }
 
-export const SOURCEMAP_POLICIES = new Set(['include', 'delete'])
+// `delete` was dropped with the code that implemented it: accepting a policy
+// nothing honours would silently ship maps that do not match the bundle.
+export const SOURCEMAP_POLICIES = new Set(['include'])
 
-/** A typo would fall through to `include` and re-inflate the patch by 5.8 MB. */
+/** An unrecognised policy is a manifest bug, not a default to fall through to. */
 export function assertSourcemapPolicy(manifest) {
   const policy = manifest.sourcemaps?.policy
   if (!SOURCEMAP_POLICIES.has(policy)) {
@@ -298,11 +293,17 @@ export function formatCheckFailure({ name, patchPath, committed, regenerated })
   ].join('\n')
 }
 
+// npm ships as `npm.cmd` on Windows, and execFile applies no PATHEXT and refuses
+// a `.cmd` target without a shell (CVE-2024-27980). git and tar are real .exe.
+const WINDOWS_SHIM_COMMANDS = new Set(['npm', 'npx', 'pnpm', 'yarn'])
+
 function run(command, args, options = {}) {
-  return execFileSync(command, args, {
+  const shim = process.platform === 'win32' && WINDOWS_SHIM_COMMANDS.has(command)
+  return execFileSync(shim ? `${command}.cmd` : command, args, {
     encoding: 'utf8',
     maxBuffer: 256 * 1024 * 1024,
     stdio: ['ignore', 'pipe', 'inherit'],
+    shell: shim,
     ...options
   })
 }
@@ -504,29 +505,6 @@ function diffFolders(folderA, folderB) {
   return normalizePnpmDiff(stdout, folderA, folderB)
 }
 
-// Why: dropping only the map hunks would ship maps whose offsets no longer line
-// up with the patched bundle. Deleting the maps outright is the honest form of
-// the same size saving, and the diff carries it as a file-deletion stanza.
-function deleteGeneratedSourcemaps(patchedDir, packageEntry) {
-  for (const relative of listFilesRelative(patchedDir)) {
-    const posix = toPosix(relative)
-    if (!packageEntry.generatedPaths.some((prefix) => posix.startsWith(prefix))) {
-      continue
-    }
-    const absolute = path.join(patchedDir, relative)
-    if (posix.endsWith('.map')) {
-      rmSync(absolute)
-      continue
-    }
-    // The reference outlives the file it points at, so it goes with it.
-    const text = readFileSync(absolute, 'utf8')
-    const stripped = text.replace(/\n\/\/# sourceMappingURL=[^\n]*\n?$/, '')
-    if (stripped !== text) {
-      writeFileSync(absolute, stripped)
-    }
-  }
-}
-
 /** The source of truth for the hand-written half: what the checkout itself holds. */
 function diffCheckoutSource(packageRoot) {
   return run('git', [...CHECKOUT_DIFF_FLAGS, '--', 'src/'], {
@@ -557,9 +535,6 @@ function regeneratePackage(packageEntry, manifest, context) {
 
   const patchedDir = path.join(workDir, 'patched', packageEntry.name.replace(/[@/]/g, '_'))
   overlayBuildOutput(pristineDir, upstreamRoot, packageEntry, patchedDir)
-  if (assertSourcemapPolicy(manifest) === 'delete') {
-    deleteGeneratedSourcemaps(patchedDir, packageEntry)
-  }
 
   // Leave the checkout diffable: the pinned commit plus the source patch, with
   // no publish-time version stamp mixed in, so `git diff` there is the source

config/scripts/regenerate-xterm-patches.test.mjs

@@ -13,7 +13,6 @@ import {
   assertSourcemapPolicy,
   firstDifferenceIndex,
   formatCheckFailure,
-  generatedHunks,
   lockfilePatchHashIsStale,
   normalizePnpmDiff,
   patchHash,
@@ -26,6 +25,15 @@ import {
   updateLockfilePatchHash
 } from './regenerate-xterm-patches.mjs'
 
+// Only the tests need to slice the generated half out of a patch; the generator
+// reads `generatedPaths` directly where it compares against the pristine build.
+function generatedHunks(patchText, generatedPaths) {
+  return splitPatchEntries(patchText)
+    .filter((entry) => generatedPaths.some((prefix) => entry.path.startsWith(prefix)))
+    .map((entry) => entry.text)
+    .join('')
+}
+
 const REPO_ROOT = path.resolve(import.meta.dirname, '..', '..')
 const MANIFEST_PATH = path.join(REPO_ROOT, 'config', 'patches', 'xterm-upstream.json')
 const temporaryDirectories = []
@@ -318,10 +326,14 @@ describe('manifest guards', () => {
   })
 
   it('refuses a sourcemap policy it does not implement', () => {
-    expect(assertSourcemapPolicy({ sourcemaps: { policy: 'delete' } })).toBe('delete')
     expect(assertSourcemapPolicy({ sourcemaps: { policy: 'include' } })).toBe('include')
+    // `delete` named a code path that no longer exists; accepting it would ship
+    // maps that do not match the bundle.
+    expect(() => assertSourcemapPolicy({ sourcemaps: { policy: 'delete' } })).toThrow(
+      /must be one of include, got "delete"/
+    )
     expect(() => assertSourcemapPolicy({ sourcemaps: { policy: 'exclude' } })).toThrow(
-      /must be one of include, delete, got "exclude"/
+      /must be one of include, got "exclude"/
     )
     expect(() => assertSourcemapPolicy({})).toThrow(/got undefined/)
   })

docs/reference/xterm-patch-regeneration.md

@@ -41,13 +41,12 @@ and are tracked separately; see [Known Gaps](#known-gaps).
    silently patch the wrong tree.
 5. Sourcemaps move with the bundle, and are never silently omitted. The patch
    moves the code, so dropping only the map hunks would ship offsets pointing at
-   the wrong lines. `sourcemaps.policy` in the manifest picks between patching
-   them (`include`, today) and deleting them (`delete`); omitting them is not an
-   option either setting offers. `include` costs about 5.8 MB of the emitted
-   patch and is required because
+   the wrong lines. `sourcemaps.policy` accepts `include` and nothing else: it
+   costs about 5.8 MB of the emitted patch and is required because
    `src/renderer/src/components/terminal-pane/terminal-ime-xterm-transaction-events.test.ts`
    reads `lib/*.map` and asserts the mapped `Version.ts` matches the runtime
-   version. `delete` becomes available again only once nothing reads them.
+   version. Deleting the maps was once an option; the code that did it was
+   removed as unreachable, so re-adding the policy means re-adding that code.
 6. `--check` is the authority on the lockfile, not `pnpm install`. pnpm writes the
    patch hash in two places — `patchedDependencies` and every resolution key that
    depends on the patched package — and on a warm store it will leave the
@@ -231,8 +230,10 @@ and which folding them into this manifest would also fix.
 Both addons do build from the pinned commit: the registry stamps
 `53a98a720ae4a973e384fa2440880d09537132f3` on `addon-webgl@0.20.0-beta.286` and
 `addon-serialize@0.15.0-beta.287` alike, despite the mismatched version numbers.
-Their published `lib/*.mjs` and `lib/*.mjs.map` reproduce byte for byte from that
-commit; this was measured, not assumed.
+On 2026-08-17 their published `lib/*.mjs` and `lib/*.mjs.map` reproduced byte for
+byte from that commit. That was a one-off measurement, not an invariant: no check
+in this repo re-runs it, so treat it as a starting point to re-measure rather than
+as something the harness holds true.
 
 What blocks folding them in is the other half of their published output. Both
 also ship a CJS `lib/addon-*.js`, and the root `package` script does not build