From 1b45d648e42685b9a8f75ccde2c467367cbc331f Mon Sep 17 00:00:00 2001 From: Merge Sim Date: Sat, 5 Sep 2026 19:31:28 -0700 Subject: [PATCH] fix(windows): make the compiled addon prove its own CreationTime support CI caught the real defect: the win32 guard test read isWindowsProcessStartTimeAvailable() as true and then found 0 rows carrying creationTimeMs. Unlike node-pty, this package publishes a prebuilt .node at the same build/Release path node-gyp writes to, so pnpm patches the source tree and leaves that binary alone. A host then holds a patched lib/index.js -- ProcessDataFlag.CreationTime and all -- over a binary that ignores flag 4, and neither a load check nor a path check can see the difference. So the binary now says so itself: addon.cc exports supportedProcessDataFlags, lib/index.js re-exports it, and - windows-process-tree-creation-time.cjs asserts it during install, which is what forces a from-source rebuild. It is shared by the Node probe in ensure-native-runtime.mjs and the Electron probe in rebuild-native-deps.mjs, exactly as node-pty-job-ownership.cjs is -- the Electron half matters because that probe decides onlyModules, so without it the packaged app would ship the stale prebuilt. - isWindowsProcessStartTimeAvailable() gates on the reported bit, not the enum. Believing the enum is worse than reporting false: the descendant snapshot returns null forever and the exit proof latches unverifiable while structured chat believes it has a reaper. rebuildNodeRuntimeModules could not actually have rebuilt this package: the patched binding.gyp includes deps/node-addon-api, which the tarball does not ship, and node-gyp must run from the physical dir. Also closes the relay repair path's divergence: repairCreationTimeSources wrote the C++ but not the buildNode splat or the tree-node typing, and assertPatchApplied checked neither, so a repaired tree passed as patched with buildProcessTree silently dropping the field. The guard test is unchanged. --- .../@vscode__windows-process-tree@0.8.0.patch | 252 ++++++++++++++++-- ...build-windows-process-tree-relay-addon.mjs | 80 +++++- config/scripts/ensure-native-runtime.mjs | 18 +- config/scripts/pr-code-change-scope.mjs | 2 + config/scripts/rebuild-native-deps.mjs | 9 + .../windows-process-tree-creation-time.cjs | 42 +++ docs/reference/windows-process-enumeration.md | 18 ++ ...claude-structured-location-support.test.ts | 3 + .../windows/windows-process-table.test.ts | 47 +++- src/main/windows/windows-process-table.ts | 33 ++- 10 files changed, 457 insertions(+), 47 deletions(-) create mode 100644 config/scripts/windows-process-tree-creation-time.cjs diff --git a/config/patches/@vscode__windows-process-tree@0.8.0.patch b/config/patches/@vscode__windows-process-tree@0.8.0.patch index fe5e4be44b1..dd7aaaee9d8 100644 --- a/config/patches/@vscode__windows-process-tree@0.8.0.patch +++ b/config/patches/@vscode__windows-process-tree@0.8.0.patch @@ -1,28 +1,30 @@ diff --git a/binding.gyp b/binding.gyp -index 855bd4b86f0a3c18c7594212c0e42b6e35bc4001..33774e7ae296f0de39dd94156673c9e773638bf4 100644 +index 855bd4b86f0a3c18c7594212c0e42b6e35bc4001..0bb2af7923b6e6f1f0da40cae8067304cd1fea14 100644 --- a/binding.gyp +++ b/binding.gyp @@ -3,7 +3,6 @@ - { - "target_name": "windows_process_tree", - "dependencies": [ -- " ({ ++ const buildNode = ({ info: { pid, name, memory, commandLine, creationTimeMs }, children }, depth) => ({ + pid, + name, + memory, + commandLine, ++ creationTimeMs, + children: depth > 0 ? children.map(c => buildNode(c, depth - 1)) : [], + }); + return buildNode(root, maxDepth); +diff --git a/lib/index.ts b/lib/index.ts +index f9aa005d9ced9e42885b8a976de5eb5bd61899ee..270967c1d1192b37cb6b095457bf52b213f88e8d 100644 +--- a/lib/index.ts ++++ b/lib/index.ts +@@ -6,12 +6,15 @@ + import { promisify } from 'util'; + + const native = process.platform === 'win32' ? require('../build/Release/windows_process_tree.node') : undefined; ++/** The flag bits this compiled addon reports; undefined off win32. */ ++export const supportedProcessDataFlags: number | undefined = native?.supportedProcessDataFlags; + import { IProcessInfo, IProcessTreeNode, IProcessCpuInfo } from '@vscode/windows-process-tree'; + + export enum ProcessDataFlag { + None = 0, + Memory = 1, +- CommandLine = 2 ++ CommandLine = 2, ++ CreationTime = 4 + } + + type RequestCallback = (processList: IProcessInfo[]) => void; +@@ -81,11 +84,12 @@ export function buildProcessTree(rootPid: number, processList: Iterable ({ ++ const buildNode = ({ info: { pid, name, memory, commandLine, creationTimeMs }, children }: IProcessInfoNode, depth: number): IProcessTreeNode => ({ + pid, + name, + memory, + commandLine, ++ creationTimeMs, + children: depth > 0 ? children.map(c => buildNode(c, depth - 1)) : [], + }); + +diff --git a/src/addon.cc b/src/addon.cc +index 9214aff281251e797a70ecb9f6e0b52932a0503f..722edd42ddb4740296bfc47582a181bd6d00c464 100644 +--- a/src/addon.cc ++++ b/src/addon.cc +@@ -53,6 +53,10 @@ void GetProcessCpuUsage(const Napi::CallbackInfo& args) { + Napi::Object Init(Napi::Env env, Napi::Object exports) { + exports.Set("getProcessList", Napi::Function::New(env, GetProcessList)); + exports.Set("getProcessCpuUsage", Napi::Function::New(env, GetProcessCpuUsage)); ++ // Lets a caller prove THIS BINARY understands CREATIONTIME. The JS enum is ++ // patched source and says nothing about what the .node was compiled from. ++ exports.Set("supportedProcessDataFlags", ++ Napi::Number::New(env, MEMORY | COMMANDLINE | CREATIONTIME)); + return exports; + } + +diff --git a/src/process.cc b/src/process.cc +index 3eea92077c4d1d433119361d5c432881859131e9..c69442e2917b8474dbabaa1aa5b2c5ce746a56e7 100644 +--- a/src/process.cc ++++ b/src/process.cc +@@ -21,7 +21,7 @@ uint32_t GetRawProcessList(std::vector& process_info, + if (Process32First(snapshot_handle, &process_entry)) { + do { + if (process_entry.th32ProcessID != 0) { +- ProcessInfo pinfo; ++ ProcessInfo pinfo{}; + pinfo.pid = process_entry.th32ProcessID; + pinfo.ppid = process_entry.th32ParentProcessID; + +@@ -33,17 +33,43 @@ uint32_t GetRawProcessList(std::vector& process_info, + GetProcessCommandLine(pinfo); + } + ++ if (CREATIONTIME & process_data_flags) { ++ GetProcessCreationTime(pinfo); ++ } ++ + strcpy(pinfo.name, process_entry.szExeFile); + process_info.push_back(std::move(pinfo)); + process_count++; + } +- } while (process_count < 1024 && Process32Next(snapshot_handle, &process_entry)); ++ } while (Process32Next(snapshot_handle, &process_entry)); + } + + CloseHandle(snapshot_handle); + return process_count; + } + ++void GetProcessCreationTime(ProcessInfo& process_info) { ++ HANDLE hProcess = OpenProcess(PROCESS_QUERY_LIMITED_INFORMATION, false, process_info.pid); +>>>>>>> 41a13099e8 (fix(windows): make the compiled addon prove its own CreationTime support) + if (hProcess == NULL) { + return; + } @@ -205,6 +329,7 @@ index 3eea92077c4d1d433119361d5c432881859131e9..738775f6fcdfb676054386fe34c03803 + CloseHandle(hProcess); +} + +<<<<<<< HEAD +// Per documentation, it is not recommended to add or subtract values from the FILETIME +// structure, or to cast it to ULARGE_INTEGER as this can cause alignment faults on 64-bit Windows. +// Copy the high and low part to a ULARGE_INTEGER and peform arithmetic on that instead. @@ -243,11 +368,56 @@ index 3eea92077c4d1d433119361d5c432881859131e9..738775f6fcdfb676054386fe34c03803 + ULONGLONG endSysTime = GetTotalTime(&sysKernelTime, &sysUserTime); + + cpu_info.cpu = 100.0 * (endProcTime - cpu_info.initialProcRunTime) / (endSysTime - cpu_info.initialSystemTime); +======= + void GetProcessMemoryUsage(ProcessInfo& process_info) { + DWORD pid = process_info.pid; + HANDLE hProcess; +diff --git a/src/process.h b/src/process.h +index 82f8e4bcfa742551e5d874a7632736a7611d7aa7..087bf4ca81768918922a1e26836c70d7aec349ec 100644 +--- a/src/process.h ++++ b/src/process.h +@@ -22,18 +22,22 @@ struct ProcessInfo { + DWORD ppid; + DWORD memory; // Reported in bytes + std::string commandLine; ++ ULONGLONG creationTimeMs; + }; + + enum ProcessDataFlags { + NONE = 0, + MEMORY = 1, +- COMMANDLINE = 2 ++ COMMANDLINE = 2, ++ CREATIONTIME = 4 + }; + + uint32_t GetRawProcessList(std::vector& process_info, DWORD flags); + + void GetProcessMemoryUsage(ProcessInfo& process_info); + ++void GetProcessCreationTime(ProcessInfo& process_info); ++ + void GetCpuUsage(Cpu& cpu_info, bool first_run); + + #endif // SRC_PROCESS_H_ +diff --git a/src/process_worker.cc b/src/process_worker.cc +index c9e3457a759c1acaa2644231a4917d45aed951f8..89f3339d5dc3b454397941c958b95965afed5788 100644 +--- a/src/process_worker.cc ++++ b/src/process_worker.cc +@@ -43,6 +43,11 @@ void GetProcessesWorker::OnOK() { + Napi::String::New(env, pinfo.commandLine)); + } + ++ if ((CREATIONTIME & process_data_flags_) && pinfo.creationTimeMs != 0) { ++ object.Set("creationTimeMs", ++ Napi::Number::New(env, static_cast(pinfo.creationTimeMs))); +>>>>>>> 41a13099e8 (fix(windows): make the compiled addon prove its own CreationTime support) + } + } else { + cpu_info.cpu = std::numeric_limits::quiet_NaN(); + } + +<<<<<<< HEAD + CloseHandle(hProcess); } \ No newline at end of file @@ -448,3 +618,49 @@ index ea822b120e8038a4803e34647042f08f4aaf5ca1..25907c0bf542bed6c72b1b462b19bcf3 + return StoreCommandLineUtf8(process_info, command_line->Buffer, + command_line->Length / sizeof(wchar_t)); +} +======= + result.Set(i, object); + } + +diff --git a/typings/windows-process-tree.d.ts b/typings/windows-process-tree.d.ts +index 08bdac2fdc5ead6f0fcfb5ee5a021e2298c7d523..2c515242d78f9d8b8e63271d86f59d1024d71604 100644 +--- a/typings/windows-process-tree.d.ts ++++ b/typings/windows-process-tree.d.ts +@@ -7,9 +7,17 @@ declare module '@vscode/windows-process-tree' { + export enum ProcessDataFlag { + None = 0, + Memory = 1, +- CommandLine = 2 ++ CommandLine = 2, ++ CreationTime = 4 + } + ++ /** ++ * The flag bits the compiled addon actually understands, or undefined off ++ * win32. `ProcessDataFlag` above is source; this is what the binary reports, ++ * so it is the only way to tell a patched build from a stale prebuilt. ++ */ ++ export const supportedProcessDataFlags: number | undefined; ++ + export interface IProcessInfo { + pid: number; + ppid: number; +@@ -24,6 +32,9 @@ declare module '@vscode/windows-process-tree' { + * The string returned is at most 512 chars, strings exceeding this length are truncated. + */ + commandLine?: string; ++ ++ /** Process creation time in Unix milliseconds. */ ++ creationTimeMs?: number; + } + + export interface IProcessCpuInfo extends IProcessInfo { +@@ -35,6 +46,7 @@ declare module '@vscode/windows-process-tree' { + name: string; + memory?: number; + commandLine?: string; ++ creationTimeMs?: number; + children: IProcessTreeNode[]; + } + +>>>>>>> 41a13099e8 (fix(windows): make the compiled addon prove its own CreationTime support) diff --git a/config/scripts/build-windows-process-tree-relay-addon.mjs b/config/scripts/build-windows-process-tree-relay-addon.mjs index 0a385e77b9c..912bbd3c174 100644 --- a/config/scripts/build-windows-process-tree-relay-addon.mjs +++ b/config/scripts/build-windows-process-tree-relay-addon.mjs @@ -98,18 +98,31 @@ function assertPatchApplied() { 'config/patches/@vscode__windows-process-tree@0.8.0.patch; run pnpm install.' ) } + // Every string the repair below can write, so a repaired tree cannot be + // declared patched while one of the pieces is silently missing. const requiredCreationTimeSources = [ ['src/process.h', 'CREATIONTIME = 4'], ['src/process.h', 'ULONGLONG creationTimeMs'], ['src/process.cc', 'GetProcessCreationTime(pinfo)'], ['src/process.cc', 'GetProcessTimes(hProcess, &creationTime'], ['src/process_worker.cc', 'object.Set("creationTimeMs"'], + ['src/addon.cc', 'exports.Set("supportedProcessDataFlags"'], ['lib/index.js', '["CreationTime"] = 4'], + ['lib/index.js', 'exports.supportedProcessDataFlags'], + ['lib/index.js', 'creationTimeMs,'], ['lib/index.ts', 'CreationTime = 4'], - ['typings/windows-process-tree.d.ts', 'creationTimeMs?: number'] + ['lib/index.ts', 'export const supportedProcessDataFlags'], + ['lib/index.ts', 'creationTimeMs,'], + ['typings/windows-process-tree.d.ts', 'creationTimeMs?: number'], + // A regex because IProcessInfo declares the same field: only the tree node + // is followed by `children`, and that is the one buildNode fills. + ['typings/windows-process-tree.d.ts', /creationTimeMs\?: number;\r?\n\s*children:/], + ['typings/windows-process-tree.d.ts', 'export const supportedProcessDataFlags'] ] for (const [relativePath, expected] of requiredCreationTimeSources) { - if (!readFileSync(join(PACKAGE_DIR, relativePath), 'utf8').includes(expected)) { + const source = readFileSync(join(PACKAGE_DIR, relativePath), 'utf8') + const present = typeof expected === 'string' ? source.includes(expected) : expected.test(source) + if (!present) { throw new Error( `${relativePath} does not contain the process creation-time patch (${expected}). ` + 'Run pnpm install before building the relay addon.' @@ -213,18 +226,51 @@ function repairCreationTimeSources() { ) }) + rewrite('src/addon.cc', (source, eol) => { + if (source.includes('exports.Set("supportedProcessDataFlags"')) { + return source + } + return source.replace( + /( exports\.Set\("getProcessCpuUsage", Napi::Function::New\(env, GetProcessCpuUsage\)\);\r?\n)/, + `$1 exports.Set("supportedProcessDataFlags",${eol}` + + ` Napi::Number::New(env, MEMORY | COMMANDLINE | CREATIONTIME));${eol}` + ) + }) + + // Each piece is guarded on its own: an early-out on the enum alone would let a + // tree with the enum but no buildNode splat pass as repaired. + const NATIVE_CONST = + "const native = process.platform === 'win32' ? require('../build/Release/windows_process_tree.node') : undefined;" for (const relativePath of ['lib/index.ts', 'lib/index.js']) { + const isTs = relativePath.endsWith('.ts') rewrite(relativePath, (source, eol) => { - if (source.includes('CreationTime')) { - return source + let next = source + if (!next.includes('CreationTime')) { + next = isTs + ? next.replace(' CommandLine = 2', ` CommandLine = 2,${eol} CreationTime = 4`) + : next.replace( + ' ProcessDataFlag[ProcessDataFlag["CommandLine"] = 2] = "CommandLine";', + ' ProcessDataFlag[ProcessDataFlag["CommandLine"] = 2] = "CommandLine";' + + `${eol} ProcessDataFlag[ProcessDataFlag["CreationTime"] = 4] = "CreationTime";` + ) } - return relativePath.endsWith('.ts') - ? source.replace(' CommandLine = 2', ` CommandLine = 2,${eol} CreationTime = 4`) - : source.replace( - ' ProcessDataFlag[ProcessDataFlag["CommandLine"] = 2] = "CommandLine";', - ' ProcessDataFlag[ProcessDataFlag["CommandLine"] = 2] = "CommandLine";' + - `${eol} ProcessDataFlag[ProcessDataFlag["CreationTime"] = 4] = "CreationTime";` - ) + if (!next.includes('supportedProcessDataFlags')) { + const reExport = isTs + ? `/** The flag bits this compiled addon reports; undefined off win32. */${eol}` + + 'export const supportedProcessDataFlags: number | undefined = native?.supportedProcessDataFlags;' + : 'exports.supportedProcessDataFlags = native === undefined ? undefined : native.supportedProcessDataFlags;' + next = next.replace(NATIVE_CONST, `${NATIVE_CONST}${eol}${reExport}`) + } + // buildNode drops any field it does not name, so the destructure and the + // splat have to move together. + next = next.replace(/(memory, commandLine)( \}, children \})/, '$1, creationTimeMs$2') + if (!/\bcreationTimeMs,/.test(next)) { + next = next.replace( + /(\r?\n)(\s*)commandLine,(\r?\n\s*children:)/, + `$1$2commandLine,$1$2creationTimeMs,$3` + ) + } + return next }) } @@ -233,6 +279,13 @@ function repairCreationTimeSources() { if (!next.includes('CreationTime = 4')) { next = next.replace(' CommandLine = 2', ` CommandLine = 2,${eol} CreationTime = 4`) } + if (!next.includes('supportedProcessDataFlags')) { + next = next.replace( + /( CreationTime = 4\r?\n \}\r?\n)/, + `$1${eol} /** The flag bits the compiled addon reports; undefined off win32. */${eol}` + + ` export const supportedProcessDataFlags: number | undefined;${eol}` + ) + } if (!next.includes('creationTimeMs?: number')) { next = next.replace( / commandLine\?: string;\r?\n/, @@ -241,6 +294,11 @@ function repairCreationTimeSources() { ` creationTimeMs?: number;${eol}` ) } + // IProcessTreeNode is the second declaration; only it is followed by children. + next = next.replace( + /( commandLine\?: string;\r?\n)( children:)/, + `$1 creationTimeMs?: number;${eol}$2` + ) return next }) return repaired diff --git a/config/scripts/ensure-native-runtime.mjs b/config/scripts/ensure-native-runtime.mjs index b2a47b99d5b..10e8426c2a5 100644 --- a/config/scripts/ensure-native-runtime.mjs +++ b/config/scripts/ensure-native-runtime.mjs @@ -2,7 +2,7 @@ import { spawnSync } from 'node:child_process' import { createRequire } from 'node:module' -import { existsSync, readFileSync } from 'node:fs' +import { existsSync, readFileSync, realpathSync } from 'node:fs' import { release } from 'node:os' import { basename, dirname, resolve } from 'node:path' import { @@ -14,6 +14,7 @@ import { const require = createRequire(import.meta.url) const { assertNodePtyJobOwnership } = require('./node-pty-job-ownership.cjs') +const { assertWindowsProcessTreeCreationTime } = require('./windows-process-tree-creation-time.cjs') const scriptPath = import.meta.filename const projectDir = resolve(import.meta.dirname, '../..') const runtime = readRuntimeArg() @@ -262,9 +263,10 @@ function loadNativeModule(moduleName) { // A bare require loads the .node addon on win32, so it catches an ABI // mismatch on its own. What it cannot catch is *which* addon loaded: the // published tarball ships a prebuilt built from unpatched source that is - // node-addon-api, so it requires cleanly and then reads every process's - // command line out of its address space. Check the binary, not the load. - require(moduleName) + // node-addon-api, so it requires cleanly, reads every process's command + // line out of its address space, and ignores the CreationTime flag. Check + // the binary on both counts, not the load. + assertWindowsProcessTreeCreationTime({ module: require(moduleName) }) if (inspectWindowsProcessTreeAddon(windowsProcessTreeAddonPath()) === 'unpatched') { throw new Error( 'the loaded addon still calls ReadProcessMemory, so it was not built from the patched ' + @@ -380,14 +382,18 @@ function getWindowsBuildNumber() { function rebuildNodeRuntimeModules(moduleNames) { for (const moduleName of moduleNames) { - const moduleDir = dirname(require.resolve(`${moduleName}/package.json`)) + let moduleDir = dirname(require.resolve(`${moduleName}/package.json`)) if (moduleName === '@vscode/windows-process-tree') { // Why before node-gyp: this module is rebuilt precisely because the // binary was the unpatched one, and pnpm materializes it unpatched often // enough that compiling the source as-is would just rebuild the same - // reader and fail the verify pass. + // reader and fail the verify pass. The patched binding.gyp then includes + // deps/node-addon-api, which the tarball does not ship, and node-gyp must + // run from the physical dir -- both reasons live in + // windows-process-tree-gyp-rebuild.mjs. ensureWindowsProcessTreeCommandLinePatch(moduleDir) stageWindowsProcessTreeNodeAddonApiHeaders(moduleDir) + moduleDir = realpathSync(moduleDir) } console.warn(`[native-runtime] Rebuilding ${moduleName} with node-gyp.`) runPnpm(['exec', 'node-gyp', 'rebuild'], { cwd: moduleDir }) diff --git a/config/scripts/pr-code-change-scope.mjs b/config/scripts/pr-code-change-scope.mjs index e75706a38af..befcb06fe1f 100644 --- a/config/scripts/pr-code-change-scope.mjs +++ b/config/scripts/pr-code-change-scope.mjs @@ -140,6 +140,8 @@ const NATIVE_RUNTIME_PREFIXES = [ 'config/scripts/ensure-native-runtime', 'config/scripts/rebuild-native-deps', 'config/scripts/node-pty-job-ownership', + 'config/scripts/windows-process-tree-creation-time', + 'config/scripts/windows-process-tree-gyp-rebuild', 'config/scripts/electron-builder-native-rebuild', 'config/patches/node-pty@', 'config/patches/@vscode__windows-process-tree' diff --git a/config/scripts/rebuild-native-deps.mjs b/config/scripts/rebuild-native-deps.mjs index 863aac850a1..d7426d8cf1d 100644 --- a/config/scripts/rebuild-native-deps.mjs +++ b/config/scripts/rebuild-native-deps.mjs @@ -567,6 +567,15 @@ function loadNativeModule(moduleName) { } return } + if (moduleName === '@vscode/windows-process-tree') { + // The tarball prebuilt loads under Electron too -- the addon is N-API, so + // a bare require proves nothing about which source it was built from. + const { assertWindowsProcessTreeCreationTime } = projectRequire( + './config/scripts/windows-process-tree-creation-time.cjs' + ) + assertWindowsProcessTreeCreationTime({ module: projectRequire(moduleName) }) + return + } projectRequire(moduleName) } diff --git a/config/scripts/windows-process-tree-creation-time.cjs b/config/scripts/windows-process-tree-creation-time.cjs new file mode 100644 index 00000000000..88f231f14d3 --- /dev/null +++ b/config/scripts/windows-process-tree-creation-time.cjs @@ -0,0 +1,42 @@ +'use strict' + +/** + * Prove the COMPILED addon understands `CREATIONTIME`, not just the patched JS. + * + * Unlike node-pty, this package ships a prebuilt `.node` at the same + * `build/Release/` path node-gyp writes to, so neither a load nor a path check + * can tell a stale prebuilt from a source build. pnpm patches the source tree + * and leaves that prebuilt in place, which is how `ProcessDataFlag.CreationTime` + * came to exist in `lib/index.js` on a binary that ignores flag 4 -- the gate + * read true and every row came back without `creationTimeMs`. + * + * `supportedProcessDataFlags` is exported by the patched `addon.cc`, so its + * presence is the binary's own answer. Shared by the Node and Electron probes + * the way `node-pty-job-ownership.cjs` is. + */ + +/** `ProcessDataFlags::CREATIONTIME` in src/process.h. */ +const CREATION_TIME_FLAG = 4 + +function assertWindowsProcessTreeCreationTime({ module, platform = process.platform }) { + if (platform !== 'win32') { + return + } + const supported = module?.supportedProcessDataFlags + if (typeof supported === 'number' && (supported & CREATION_TIME_FLAG) !== 0) { + return + } + throw new Error( + [ + '@vscode/windows-process-tree does not report CreationTime support', + `(supportedProcessDataFlags=${String(supported)}).`, + 'That is the tarball prebuilt, not a build of the patched source, so every', + 'process row comes back without creationTimeMs: Windows descendant exit', + 'verification cannot identify a PID and structured Claude/Codex chat runs', + 'with an unprovable child-tree reaper.', + 'Rebuild it from source so config/patches/@vscode__windows-process-tree@0.8.0.patch applies.' + ].join(' ') + ) +} + +module.exports = { assertWindowsProcessTreeCreationTime, CREATION_TIME_FLAG } diff --git a/docs/reference/windows-process-enumeration.md b/docs/reference/windows-process-enumeration.md index 0aaeab54875..0f7f17bd433 100644 --- a/docs/reference/windows-process-enumeration.md +++ b/docs/reference/windows-process-enumeration.md @@ -368,6 +368,24 @@ on any other OS keeps using the scan. to Unix ms; a process that denies the handle is emitted with the field absent, never zero, because callers must be able to tell "cannot identify" from a timestamp. +5. **`supportedProcessDataFlags`.** `addon.cc` exports the flag bits the + compiled binary understands, and `lib/index.js` re-exports it. + + Why a fifth hunk and not just the enum: unlike `node-pty`, this package + publishes a prebuilt `.node` at the same `build/Release/` path node-gyp + writes to. pnpm patches the source tree and leaves that prebuilt alone, so a + host can hold a patched `lib/index.js` — `ProcessDataFlag.CreationTime` and + all — over a binary that ignores flag 4. CI produced exactly that: the gate + read available and every row came back without `creationTimeMs`. Neither a + load check nor a path check can see the difference, so the binary has to say + so itself. + + Two readers depend on it. `isWindowsProcessStartTimeAvailable()` returns + false unless this bit is set, because claiming otherwise leaves + `captureWindowsDescendantSnapshot` returning null forever while structured + chat believes it has a reaper. And `windows-process-tree-creation-time.cjs` + asserts it during install, which is what forces a from-source rebuild — + the same role `node-pty-job-ownership.cjs` plays for node-pty's job exports. The typings claim `commandLine` is truncated at 512 characters. Measured, it is not: the longest observed on a real host was 26,059. diff --git a/src/main/claude/claude-structured-location-support.test.ts b/src/main/claude/claude-structured-location-support.test.ts index 1106667d544..fe3820964e3 100644 --- a/src/main/claude/claude-structured-location-support.test.ts +++ b/src/main/claude/claude-structured-location-support.test.ts @@ -56,8 +56,11 @@ describe('supportsClaudeStructuredLocation', () => { it('accepts Windows local locations once creation-time proof is available', () => { previousPlatform = setPlatform('win32') + // supportedProcessDataFlags is the addon's own report; the enum alone is + // not proof, because pnpm patches the source over the tarball's prebuilt. __setWindowsProcessTreeLoaderForTests(() => ({ ProcessDataFlag: { None: 0, Memory: 1, CommandLine: 2, CreationTime: 4 }, + supportedProcessDataFlags: 7, getAllProcesses: () => undefined })) expect( diff --git a/src/main/windows/windows-process-table.test.ts b/src/main/windows/windows-process-table.test.ts index d8d15b3788e..16c411ceb71 100644 --- a/src/main/windows/windows-process-table.test.ts +++ b/src/main/windows/windows-process-table.test.ts @@ -149,6 +149,7 @@ describe('windows process table', () => { Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' }) __setWindowsProcessTreeLoaderForTests(() => ({ ProcessDataFlag: { None: 0, Memory: 1, CommandLine: 2, CreationTime: 4 }, + supportedProcessDataFlags: 7, getAllProcesses })) }) @@ -327,10 +328,23 @@ describe('windows process table', () => { vi.useRealTimers() }) - it('only advertises PID-safe ownership when the native creation-time field exists', () => { + it('only advertises PID-safe ownership when the BINARY reports creation-time support', () => { expect(isWindowsProcessStartTimeAvailable()).toBe(true) + + // The shape CI produced: pnpm patched the source tree, so the enum carries + // CreationTime, while the tarball's prebuilt .node still ignores flag 4. + // Believing the enum here is what let structured chat run with a reaper + // that can never identify a PID. __setWindowsProcessTreeLoaderForTests(() => ({ - ProcessDataFlag: { None: 0, Memory: 1, CommandLine: 2 }, + ProcessDataFlag: { None: 0, Memory: 1, CommandLine: 2, CreationTime: 4 }, + supportedProcessDataFlags: 3, + getAllProcesses + })) + expect(isWindowsProcessStartTimeAvailable()).toBe(false) + + // An addon predating the export at all reports nothing, which is also false. + __setWindowsProcessTreeLoaderForTests(() => ({ + ProcessDataFlag: { None: 0, Memory: 1, CommandLine: 2, CreationTime: 4 }, getAllProcesses })) expect(isWindowsProcessStartTimeAvailable()).toBe(false) @@ -654,12 +668,21 @@ describe('resolving the native reader', () => { } }) - function addonReturning(rows: unknown): { getProcessList: ReturnType } { + function addonReturning(rows: unknown): { + getProcessList: ReturnType + supportedProcessDataFlags: number + } { return { - getProcessList: vi.fn((cb: (r: unknown) => void) => cb(rows)) + getProcessList: vi.fn((cb: (r: unknown) => void) => cb(rows)), + supportedProcessDataFlags: 7 } } + /** An addon built before the creation-time patch: no capability export at all. */ + function staleAddonReturning(rows: unknown): { getProcessList: ReturnType } { + return { getProcessList: vi.fn((cb: (r: unknown) => void) => cb(rows)) } + } + it('prefers the npm package where the desktop app installs it', async () => { const resolve = vi.fn((specifier: string) => { if (specifier === PACKAGE_SPECIFIER) { @@ -726,6 +749,22 @@ describe('resolving the native reader', () => { expect(addon.getProcessList).toHaveBeenCalledWith(expect.any(Function), 4) }) + it('trusts the staged addon on its own report, not on ours', async () => { + // A relay carrying an addon built before the creation-time patch still + // enumerates, so the table stays usable -- but it cannot prove identity, + // and saying otherwise would hand teardown a PID it can never re-check. + const addon = staleAddonReturning(NATIVE) + __setWindowsProcessTreeRequireForTests((specifier: string) => { + if (specifier === ADDON_SPECIFIER) { + return addon + } + throw new Error('MODULE_NOT_FOUND') + }) + await expect(readWindowsProcessTableFresh()).resolves.toHaveLength(2) + expect(isWindowsProcessTableAvailable()).toBe(true) + expect(isWindowsProcessStartTimeAvailable()).toBe(false) + }) + it('reaches the CIM scan when neither the package nor the addon is present', async () => { const cimScan = vi .fn() diff --git a/src/main/windows/windows-process-table.ts b/src/main/windows/windows-process-table.ts index 467ebab31cc..e3064560174 100644 --- a/src/main/windows/windows-process-table.ts +++ b/src/main/windows/windows-process-table.ts @@ -75,6 +75,13 @@ type WindowsProcessTreeModule = { CommandLine: number CreationTime?: number } + /** + * Flag bits the COMPILED addon reports, straight from `addon.cc`. Absent on a + * build that predates the patch — which is not the same question as the enum + * above, because pnpm patches the source tree and leaves the tarball's + * prebuilt `.node` in place. + */ + supportedProcessDataFlags?: number getAllProcesses: ( callback: (processes: NativeProcessInfo[] | undefined) => void, flags?: number @@ -109,6 +116,7 @@ type WindowsProcessTreeAddon = { callback: (processes: NativeProcessInfo[] | undefined) => void, flags: number ) => void + supportedProcessDataFlags?: number } /** @@ -116,10 +124,8 @@ type WindowsProcessTreeAddon = { * is listed for completeness and is deliberately never set — see the projections * below. * - * Why `CreationTime` can be named here rather than probed: the bare addon is a - * content-hashed relay artifact, so it ships in the same immutable relay - * directory as the bundle reading it and can never be an older build than the - * code asking for the bit. + * Naming `CreationTime` here only decides what we ASK for; whether the binary + * answers is `supportedProcessDataFlags`, which the addon reports itself. */ const PROCESS_DATA_FLAG = { None: 0, Memory: 1, CommandLine: 2, CreationTime: 4 } as const @@ -170,6 +176,7 @@ let cimScan: () => Promise = readWindowsProcessRowsWithCim function adaptAddon(addon: WindowsProcessTreeAddon): WindowsProcessTreeModule { return { ProcessDataFlag: PROCESS_DATA_FLAG, + supportedProcessDataFlags: addon.supportedProcessDataFlags, getAllProcesses: (callback, flags) => addon.getProcessList(callback, flags ?? 0) } } @@ -502,13 +509,23 @@ export function isWindowsProcessTableAvailable(): boolean { /** * PID-reuse-safe ownership needs the native creation-time field, not merely a - * process list. An install whose pnpm patch never applied exposes the table - * without that field; keep structured ownership unavailable on those hosts - * instead of fabricating proof from a PID. + * process list. + * + * Why the binary's own answer and not the enum: pnpm patches the package's + * source tree but leaves the tarball's prebuilt `.node` at the same + * `build/Release/` path, so a host can hold a patched `lib/index.js` — enum and + * all — over a binary that ignores flag 4. CI produced exactly that: the enum + * said available, and every row came back without `creationTimeMs`. Answering + * true there is worse than answering false: the descendant snapshot then + * returns null forever and the exit proof latches `unverifiable`, while + * structured chat believes it has a reaper. */ export function isWindowsProcessStartTimeAvailable(): boolean { const native = moduleLoader() - return native !== null && typeof native.ProcessDataFlag.CreationTime === 'number' + return ( + native !== null && + ((native.supportedProcessDataFlags ?? 0) & PROCESS_DATA_FLAG.CreationTime) !== 0 + ) } function resetSnapshotReaders(): void {