diff --git a/.claude/launch.json b/.claude/launch.json new file mode 100644 index 00000000..04ed7860 --- /dev/null +++ b/.claude/launch.json @@ -0,0 +1,14 @@ +{ + "version": "0.0.1", + "configurations": [ + { + "name": "dev", + "runtimeExecutable": "npm", + "runtimeArgs": [ + "run", + "dev" + ], + "port": 3000 + } + ] +} diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index de544dc9..0d93b94d 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -9,6 +9,15 @@ on: - main workflow_dispatch: inputs: + target: + description: Platform to build + required: false + default: all + type: choice + options: + - all + - windows + - macos windows_prerelease_tag: description: Optional non-v tag for a Windows Dev pre-release required: false @@ -24,6 +33,9 @@ concurrency: jobs: macos-apple-silicon: name: macOS Apple Silicon + if: >- + startsWith(github.ref, 'refs/tags/v') || + (github.event_name == 'workflow_dispatch' && (inputs.target == 'all' || inputs.target == 'macos')) runs-on: macos-15 steps: - uses: actions/checkout@v4 @@ -154,7 +166,9 @@ jobs: macos-intel: name: macOS Intel - if: startsWith(github.ref, 'refs/tags/v') || github.event_name == 'workflow_dispatch' + if: >- + startsWith(github.ref, 'refs/tags/v') || + (github.event_name == 'workflow_dispatch' && (inputs.target == 'all' || inputs.target == 'macos')) runs-on: macos-15-intel steps: - uses: actions/checkout@v4 @@ -285,7 +299,10 @@ jobs: windows-x64: name: Windows x64 - if: startsWith(github.ref, 'refs/tags/v') || github.event_name == 'workflow_dispatch' + if: >- + startsWith(github.ref, 'refs/tags/v') || + github.event_name == 'pull_request' || + (github.event_name == 'workflow_dispatch' && (inputs.target == 'all' || inputs.target == 'windows' || inputs.target == '' || inputs.windows_prerelease_tag != '')) runs-on: windows-2022 steps: - uses: actions/checkout@v4 @@ -305,10 +322,10 @@ jobs: if: startsWith(github.ref, 'refs/tags/v') run: npm run package:win - name: Build isolated Windows development package - if: github.event_name == 'workflow_dispatch' + if: ${{ !startsWith(github.ref, 'refs/tags/v') }} run: npm run package:dev:win - name: Smoke test packaged Windows Harness - if: github.event_name == 'workflow_dispatch' + if: ${{ !startsWith(github.ref, 'refs/tags/v') }} shell: pwsh run: | $userData = Join-Path $env:APPDATA 'dsh-desktop-dev' @@ -414,7 +431,7 @@ jobs: dist/latest.yml if-no-files-found: error - uses: actions/upload-artifact@v4 - if: github.event_name == 'workflow_dispatch' + if: ${{ !startsWith(github.ref, 'refs/tags/v') }} with: name: windows-x64-dev path: | diff --git a/build/harness-node-entry.mjs b/build/harness-node-entry.mjs index c86fbd90..0e096ee0 100644 --- a/build/harness-node-entry.mjs +++ b/build/harness-node-entry.mjs @@ -1,5 +1,18 @@ import { pathToFileURL } from 'node:url' +// On macOS Harness runs inside an Electron utility process (TCC responsibility +// isolation), so `process.execPath` and `argv0` point at the Electron helper +// instead of a Node binary. Plugins re-invoke the dsh CLI through the +// executable running them — dsh-market forwards `process.execArgv` with it — +// and without Node mode that child boots as an Electron app, where the leading +// `--expose-internals` shifts argv and the CLI answers "--profile is +// required" instead of installing. Declaring it here, after this process has +// already parsed the Chromium switches it was launched with, marks only the +// children as Node processes. Bundled-Node hosts (Windows, Linux) skip it. +if (process.versions.electron !== undefined) { + process.env.ELECTRON_RUN_AS_NODE = '1' +} + const [dshEntryPath, ...dshArguments] = process.argv.slice(2) function report(label, value) { diff --git a/packages/dsh-desktop-market-installer/client.js b/packages/dsh-desktop-market-installer/client.js index 6391f752..2176637a 100644 --- a/packages/dsh-desktop-market-installer/client.js +++ b/packages/dsh-desktop-market-installer/client.js @@ -501,7 +501,7 @@ window.__ModuleLoader__.load({ setStatus((current) => ({ ...current, phase: 'installing', - recommendedVersion: current?.recommendedVersion || '1.9.0' + recommendedVersion: current?.recommendedVersion || '1.15.0' })) try { const response = await fetch(INSTALL_PATH, { @@ -539,7 +539,7 @@ window.__ModuleLoader__.load({ const busy = phase === 'installing' const installed = phase === 'installed' const failed = phase === 'error' || phase === 'incomplete' || Boolean(error) - const version = status?.recommendedVersion || '1.9.0' + const version = status?.recommendedVersion || '1.15.0' return React.createElement( 'section', diff --git a/packages/dsh-desktop-market-installer/index.js b/packages/dsh-desktop-market-installer/index.js index eff358c7..0bd7728a 100644 --- a/packages/dsh-desktop-market-installer/index.js +++ b/packages/dsh-desktop-market-installer/index.js @@ -1,11 +1,15 @@ import { spawn } from 'node:child_process' import { existsSync } from 'node:fs' -import { chmod, mkdir, readFile, readdir, rename, rm, writeFile } from 'node:fs/promises' +import { chmod, copyFile, mkdir, readFile, readdir, rename, rm, writeFile } from 'node:fs/promises' import { createRequire } from 'node:module' import { homedir } from 'node:os' import { delimiter, dirname, join, resolve } from 'node:path' +import { fileURLToPath } from 'node:url' -export const RECOMMENDED_MARKET_VERSION = '1.9.0' +import { SIDELINE_MARKER } from './pnpm-runner.mjs' +import { removeTree } from './remove-tree.mjs' + +export const RECOMMENDED_MARKET_VERSION = '1.15.0' export const MARKET_PACKAGE = 'dshmarket' export const MARKET_PROFILE = 'web' export const STATUS_PATH = '/dsh-desktop/market-installer/status' @@ -26,19 +30,40 @@ export function profileDirectory(home = dshHome()) { return join(home, 'profiles', MARKET_PROFILE) } +/** Leftovers of an interrupted pnpm run, or of a Windows locked-rename recovery. */ +export function isDisposableModuleDirectory(name) { + return name.includes('_tmp_') || name.includes(SIDELINE_MARKER) +} + +/** + * Sweep the leftovers of interrupted pnpm runs from a profile's node_modules. + * + * A package's own node_modules is swept too. Once the package being replaced + * is a dependency of a dependency, that is where the leftovers land — + * `cytoscape-fcose/node_modules/cose-base.dsh-old-…` — and a sweep that stops + * at the top level leaves one copy behind per attempt. + */ export async function cleanStaleTemporaryDirectories(home = dshHome()) { const directory = profileDirectory(home) - const nodeModulesPath = join(directory, 'node_modules') - try { - const entries = await readdir(nodeModulesPath, { withFileTypes: true }) + const sweep = async (nodeModulesPath) => { + let entries + try { + entries = await readdir(nodeModulesPath, { withFileTypes: true }) + } catch { + // node_modules directory may not exist yet + return + } for (const entry of entries) { - if (entry.isDirectory() && entry.name.includes('_tmp_')) { - await rm(join(nodeModulesPath, entry.name), { recursive: true, force: true }).catch(() => undefined) + if (!entry.isDirectory() || entry.isSymbolicLink()) continue + const path = join(nodeModulesPath, entry.name) + if (isDisposableModuleDirectory(entry.name)) { + await removeTree(path).catch(() => undefined) + continue } + await sweep(entry.name.startsWith('@') ? path : join(path, 'node_modules')) } - } catch { - // node_modules directory may not exist yet } + await sweep(join(directory, 'node_modules')) } function readObject(text) { @@ -128,37 +153,73 @@ export function resolvePnpmEntry(requireFrom = import.meta.url) { return entry } +/** + * Put the lock-recovery runner where the shims can invoke it. + * @returns the staged runner path, or undefined when neither the staged copy + * nor the packaged original can be used — the shims then call pnpm directly. + */ +export async function stagePnpmRunner(directory) { + const source = fileURLToPath(new URL('./pnpm-runner.mjs', import.meta.url)) + const staged = join(directory, 'pnpm-runner.mjs') + try { + await copyFile(source, staged) + return staged + } catch { + return existsSync(source) ? source : undefined + } +} + export async function ensurePnpmShim(home = dshHome()) { const directory = join(home, '.desktop-bin') await mkdir(directory, { recursive: true }) const pnpmEntry = resolvePnpmEntry() const executable = process.execPath + // pnpm is reached through this shim by every profile package operation — + // DSH Desktop's installer and the community market alike — so the runner it + // points at is where a Windows locked rename gets recovered for both. A + // runner that cannot be staged must not take the shims down with it: pnpm + // still has to be reachable, just without the recovery, and the harness log + // has to say so rather than leaving a stale shim to be mistaken for a fresh + // one. + const runnerPath = await stagePnpmRunner(directory) + const pnpmCommand = runnerPath === undefined ? [pnpmEntry] : [runnerPath, pnpmEntry] + process.stdout.write( + runnerPath === undefined + ? 'dsh-desktop: pnpm shim written without the lock-recovery runner\n' + : `dsh-desktop: pnpm shim written via ${runnerPath}\n` + ) + + // The packaged executable is Electron on macOS, where Harness runs as a + // utility process. Anything invoked through these shims expects Node + // semantics — a leading node flag included — so the shims declare Node mode + // themselves instead of relying on the caller's environment. The real Node + // runtime bundled on the other platforms ignores the variable. if (process.platform === 'win32') { const pnpmPath = join(directory, 'pnpm.cmd') await writeFile( pnpmPath, - `@chcp 65001 >nul\r\n@echo off\r\n\"${executable}\" \"${pnpmEntry}\" %*\r\n`, + `@chcp 65001 >nul\r\n@echo off\r\n@set ELECTRON_RUN_AS_NODE=1\r\n\"${executable}\" ${pnpmCommand.map((part) => `\"${part}\"`).join(' ')} %*\r\n`, 'utf8' ) const nodePath = join(directory, 'node.cmd') await writeFile( nodePath, - `@chcp 65001 >nul\r\n@echo off\r\n\"${executable}\" %*\r\n`, + `@chcp 65001 >nul\r\n@echo off\r\n@set ELECTRON_RUN_AS_NODE=1\r\n\"${executable}\" %*\r\n`, 'utf8' ) } else { const pnpmPath = join(directory, 'pnpm') await writeFile( pnpmPath, - `#!/bin/sh\nexec ${shellQuote(executable)} ${shellQuote(pnpmEntry)} \"$@\"\n`, + `#!/bin/sh\nexport ELECTRON_RUN_AS_NODE=1\nexec ${shellQuote(executable)} ${pnpmCommand.map(shellQuote).join(' ')} \"$@\"\n`, { encoding: 'utf8', mode: 0o755 } ) await chmod(pnpmPath, 0o755) const nodePath = join(directory, 'node') await writeFile( nodePath, - `#!/bin/sh\nexec ${shellQuote(executable)} \"$@\"\n`, + `#!/bin/sh\nexport ELECTRON_RUN_AS_NODE=1\nexec ${shellQuote(executable)} \"$@\"\n`, { encoding: 'utf8', mode: 0o755 } ) await chmod(nodePath, 0o755) diff --git a/packages/dsh-desktop-market-installer/pnpm-runner.mjs b/packages/dsh-desktop-market-installer/pnpm-runner.mjs new file mode 100644 index 00000000..cf88a9d9 --- /dev/null +++ b/packages/dsh-desktop-market-installer/pnpm-runner.mjs @@ -0,0 +1,407 @@ +/** + * The pnpm entry every profile package operation goes through — DSH Desktop's + * own installer and the community market alike, because both reach pnpm by + * name and the packaged shim in `.desktop-bin` points here. + * + * Windows cannot replace a directory while something holds a handle inside it, + * and pnpm finishes each package by renaming `_tmp__` onto + * ``. With Harness running — it has the profile's modules loaded, and the + * platform's own scanners open files behind everyone's back — that final + * rename fails: + * + * EPERM: operation not permitted, rename '…\argparse_tmp_19856_4' -> '…\argparse' + * + * Two recoveries, in order of how little they disturb: retry once (a scanner's + * handle is gone within a second), then move the blocked target aside and + * retry (a rename of the directory itself succeeds where replacing its + * contents does not, and pnpm recreates the package under the free name). The + * leftovers are swept before Harness next starts, when nothing holds them. + * + * Anything unrecognized is passed straight through: same exit code, same + * output, one pnpm run. + */ +import { spawn } from 'node:child_process' +import { existsSync, watch } from 'node:fs' +import { readdir, rename } from 'node:fs/promises' +import { join } from 'node:path' +import { fileURLToPath, pathToFileURL } from 'node:url' + +export const SIDELINE_MARKER = '.dsh-old-' +export const RETRY_DELAY_MS = 750 +/** + * How long a run may show no sign of life — no output, no change under the + * profile — before this runner stops it. Generous on purpose: pnpm without a + * TTY says nothing for long stretches, and packages download into a store that + * lives outside the profile, so a quiet minute is ordinary. Five is not, and + * failing there still beats the hosts' fifteen-minute ceiling, which is what + * makes a wedged install feel like a hang. + */ +export const IDLE_TIMEOUT_MS = Number(process.env.DSH_DESKTOP_PNPM_IDLE_TIMEOUT_MS) || 300_000 +/** How long a killed run may take to actually go away before it is written off. */ +export const KILL_GRACE_MS = 5_000 +/** + * How long a run gets to exit on its own after it has already reported the + * failure that decides it. pnpm raises the locked rename inside a worker and + * has been seen never to unwind from it, so the outcome is known long before + * the process admits it — waiting out the idle allowance there is pure delay. + */ +export const STALL_AFTER_FAILURE_MS = + Number(process.env.DSH_DESKTOP_PNPM_FAILURE_STALL_MS) || 20_000 +/** Prefix of every line this runner contributes to a package operation's output. */ +export const MARKER = 'dsh-desktop pnpm runner:' + +function errorText(error) { + return error instanceof Error ? error.message : String(error) +} + +/** + * The destination pnpm could not claim, or undefined when the failure is not a + * locked rename inside a profile's node_modules. Only a path under + * node_modules qualifies: a rename failure anywhere else is not ours to + * rearrange. + * @param {string} output - the failed run's combined stdout and stderr. + * @returns {string | undefined} the blocked destination path. + */ +export function lockedRenameTarget(output) { + const failure = + /(?:EPERM|EBUSY|EACCES|ENOTEMPTY|EEXIST)[^\n]*?rename[^\n]*?->\s*'([^']+)'/u.exec(output) + if (failure === null) return undefined + const target = failure[1] + const inModules = target.split(/[\\/]/u).includes('node_modules') + return inModules ? target : undefined +} + +/** + * Where a blocked directory is moved so pnpm can claim the name it wants: the + * same path under a suffixed name, which keeps it a sibling without parsing + * separators the host may not own. The marker is what the pre-launch sweep + * looks for. + * @param {string} target - the blocked destination path. + * @param {number} now - timestamp making the name unique across attempts. + * @returns {string} the sideline path. + */ +export function sidelinePath(target, now = Date.now()) { + return `${target}${SIDELINE_MARKER}${now}` +} + +function delay(milliseconds) { + return new Promise((resolve) => setTimeout(resolve, milliseconds)) +} + +/** + * Run pnpm once, mirroring its streams to this process while keeping a copy + * for failure classification. + * + * A run that has stopped doing anything is stopped rather than waited out — + * the hosts above only bound the whole operation at fifteen minutes, which is + * what makes a wedged install look like a hang. But silence alone does not + * mean stuck: without a TTY pnpm drops its progress display, and resolution, + * a cold download or a large link phase can pass without a single line. What + * a live run cannot do is leave the profile untouched, so the store and the + * profile's node_modules count as much as output does. + */ +function runPnpm(executable, args, options = {}) { + const { + spawnProcess = spawn, + idleTimeoutMs = IDLE_TIMEOUT_MS, + killGraceMs = KILL_GRACE_MS, + stallAfterFailureMs = STALL_AFTER_FAILURE_MS, + kill = killTree, + watchActivity = watchProfileActivity, + report = () => undefined + } = options + + return new Promise((resolve, reject) => { + const child = spawnProcess(executable, args, { + stdio: ['inherit', 'pipe', 'pipe'], + windowsHide: true + }) + let output = '' + let idle + let grace + let stopped = false + let settled = false + let doomed = false + let lastActivity = Date.now() + const touch = () => { + lastActivity = Date.now() + } + const stopWatching = idleTimeoutMs > 0 ? watchActivity(touch) : undefined + + const finish = (result) => { + if (settled) return + settled = true + clearTimeout(idle) + clearTimeout(grace) + stopWatching?.() + resolve({ ...result, output, idleTimedOut: stopped }) + } + const heartbeat = () => { + clearTimeout(idle) + if (idleTimeoutMs <= 0) return + const quietFor = Date.now() - lastActivity + // Work seen since the last check keeps the run: a quiet pnpm that is + // still writing packages is slow, not stuck. + idle = setTimeout(() => { + const silence = Date.now() - lastActivity + if (silence < idleTimeoutMs) { + heartbeat() + return + } + stopped = true + report( + `pnpm produced no output and touched nothing for ${Math.round( + silence / 1000 + )}s; stopping it` + ) + kill(child) + // A kill that does not land must not become the hang this guards + // against, so the run is written off either way. + grace = setTimeout(() => finish({ code: 1, signal: null }), killGraceMs) + grace.unref?.() + }, Math.max(idleTimeoutMs - quietFor, 1_000)) + idle.unref?.() + } + const stopWhenDoomed = () => { + if (doomed || lockedRenameTarget(output) === undefined) return + doomed = true + report( + `pnpm reported a blocked rename; giving it ${Math.round( + stallAfterFailureMs / 1000 + )}s to exit before stopping it` + ) + const stall = setTimeout(() => { + if (settled) return + stopped = true + kill(child) + grace = setTimeout(() => finish({ code: 1, signal: null }), killGraceMs) + grace.unref?.() + }, stallAfterFailureMs) + stall.unref?.() + } + const observe = (chunk, stream) => { + output = `${output}${chunk}`.slice(-256 * 1024) + stream.write(chunk) + touch() + // pnpm can raise the locked rename in a worker and then never unwind. + // The outcome is already decided, so the run is not owed the full idle + // allowance from here. + stopWhenDoomed() + } + + child.stdout.on('data', (chunk) => observe(chunk, process.stdout)) + child.stderr.on('data', (chunk) => observe(chunk, process.stderr)) + child.once('error', (error) => { + clearTimeout(idle) + clearTimeout(grace) + stopWatching?.() + if (!settled) { + settled = true + reject(error) + } + }) + child.once('exit', (code, signal) => finish({ code: stopped ? 1 : code, signal })) + heartbeat() + }) +} + +/** + * Signal progress whenever the profile's package directories change. pnpm runs + * with the profile as its working directory, so that is where a live install + * shows up even while it says nothing. + * @param onActivity - called on any change; may fire often, so it stays cheap. + * @returns a function that stops watching. + */ +function watchProfileActivity(onActivity, cwd = process.cwd()) { + const watchers = [] + for (const directory of [cwd, join(cwd, 'node_modules'), join(cwd, 'node_modules', '.pnpm')]) { + try { + if (!existsSync(directory)) continue + const watcher = watch(directory, { persistent: false }, onActivity) + watcher.on('error', () => undefined) + watchers.push(watcher) + } catch { + // An unwatchable directory just does not contribute liveness. + } + } + return () => { + for (const watcher of watchers) watcher.close() + } +} + +/** + * Stop a pnpm run and everything it started. On Windows `kill` reaches only + * the wrapper, so the tree goes through taskkill — but never *instead of* the + * direct kill: a taskkill that does not land would leave this runner waiting + * on an exit that never comes, which is the hang it exists to prevent. + */ +function killTree(child) { + if (process.platform === 'win32' && child.pid !== undefined) { + try { + spawn('taskkill', ['/pid', String(child.pid), '/t', '/f'], { + stdio: 'ignore', + windowsHide: true + }) + } catch { + // The direct kill below is the guarantee. + } + } + child.kill('SIGKILL') +} + +/** + * Run pnpm, recovering from a Windows locked rename. Returns the exit code of + * the run that decided the outcome. + */ +export async function runWithLockRecovery(executable, args, options = {}) { + const { + spawnProcess = spawn, + moveAside = rename, + exists = existsSync, + listEntries = readEntries, + // pnpm runs with the profile as its working directory, so that is where + // the staging left by a failed run is found. + profileDirectory = process.cwd(), + wait = delay, + now = Date.now, + retryDelayMs = RETRY_DELAY_MS, + idleTimeoutMs = IDLE_TIMEOUT_MS, + killGraceMs = KILL_GRACE_MS, + stallAfterFailureMs = STALL_AFTER_FAILURE_MS, + kill = killTree, + watchActivity = watchProfileActivity, + // Every step announces itself on the same stream pnpm's own diagnostics + // travel, because the market reports that stream verbatim: a report + // without these lines is a report from a pnpm this runner never wrapped. + report = (message) => process.stderr.write(`${MARKER} ${message}\n`) + } = options + + const run = () => + runPnpm(executable, args, { + spawnProcess, + idleTimeoutMs, + killGraceMs, + stallAfterFailureMs, + kill, + watchActivity, + report + }) + + const first = await run() + const blocked = first.code === 0 ? undefined : lockedRenameTarget(first.output) + // Whether the run exited on its own or had to be stopped says nothing about + // whether the blocked rename can be recovered — and a run that names its + // blocker before hanging is exactly the one this recovery is for. Only a + // failure with no diagnosis is left alone: retrying that is three times the + // wait for the same answer. + if (blocked === undefined) return first + + report(`${blocked} could not be replaced; retrying in ${retryDelayMs}ms (2 of 3)`) + await wait(retryDelayMs) + const second = await run() + const named = second.code === 0 ? undefined : lockedRenameTarget(second.output) + if (named === undefined) { + if (second.code === 0) report('the retry succeeded') + return second + } + + // pnpm names one blocked destination per run — the first its workers hit — + // but an update blocks on every package it has to replace. Freeing only the + // named one buys a single package per attempt, so an update that replaces + // four of them can never finish inside three. Every destination pnpm staged + // for is already on disk next to its `_tmp__`, so the whole set + // is knowable from one failure, and the whole set is what gets freed here. + const targets = await blockedTargets(named, profileDirectory, { listEntries, exists }) + const freed = [] + for (const target of targets) { + const sideline = sidelinePath(target, now()) + try { + await moveAside(target, sideline) + freed.push(target) + } catch (error) { + // The directory itself is held too — nothing left to try for this one, + // and the run's own diagnostics are already on stderr. + report(`${target} could not be moved aside (${errorText(error)})`) + } + } + + if (freed.length === 0) { + report(`nothing could be freed; leaving pnpm's own diagnosis in place`) + return second + } + report( + `freed ${freed.length} blocked ${ + freed.length === 1 ? 'destination' : 'destinations' + } (${freed.join(', ')}); installing over them (3 of 3)` + ) + const third = await run() + report(third.code === 0 ? 'the install succeeded' : 'the install failed again') + return third +} + +/** The `_tmp__` staging name pnpm leaves beside its destination. */ +const STAGING_PATTERN = /^(?.+)_tmp_\d+_\d+$/u + +/** + * Every destination the failed run still has staging for, plus the one pnpm + * named. A staging directory sits beside the destination it was built for, so + * stripping the suffix names that destination without parsing pnpm's output + * twice — and it finds the ones pnpm never got far enough to report. + * + * Nested node_modules are walked because a replaced dependency of a dependency + * stages there, not at the top level. + * @param named - the destination pnpm reported, always included when present. + * @param root - the profile directory pnpm ran in. + * @returns absolute destination paths, deduplicated, each one present on disk. + */ +export async function blockedTargets(named, root, options = {}) { + const { listEntries = readEntries, exists = existsSync } = options + const found = new Set() + + const walk = async (directory) => { + for (const entry of await listEntries(directory)) { + if (!entry.isDirectory() || entry.isSymbolicLink()) continue + const path = join(directory, entry.name) + const staged = STAGING_PATTERN.exec(entry.name) + if (staged !== null) { + found.add(join(directory, staged.groups.packageName)) + continue + } + await walk(entry.name.startsWith('@') ? path : join(path, 'node_modules')) + } + } + await walk(join(root, 'node_modules')) + + // pnpm's own diagnosis leads, because it names what actually blocked the run. + const ordered = [named, ...found].filter((path) => path !== undefined) + return [...new Set(ordered)].filter((path) => exists(path)) +} + +async function readEntries(directory) { + try { + return await readdir(directory, { withFileTypes: true }) + } catch { + return [] + } +} + +/* v8 ignore start -- the process wrapper around the tested runner */ +if (process.argv[1] !== undefined && import.meta.url === pathToFileURL(process.argv[1]).href) { + const [pnpmEntry, ...pnpmArguments] = process.argv.slice(2) + if (pnpmEntry === undefined) { + process.stderr.write('dsh-desktop: the pnpm runner needs the pnpm entry path.\n') + process.exitCode = 1 + } else { + const executable = process.execPath + const result = await runWithLockRecovery(executable, [pnpmEntry, ...pnpmArguments]) + if (result.signal) { + process.stderr.write(`dsh-desktop: pnpm terminated with ${result.signal}.\n`) + process.exitCode = 1 + } else { + process.exitCode = result.code ?? 1 + } + } +} +/* v8 ignore stop */ + +export const RUNNER_PATH = fileURLToPath(import.meta.url) diff --git a/packages/dsh-desktop-market-installer/remove-tree.mjs b/packages/dsh-desktop-market-installer/remove-tree.mjs new file mode 100644 index 00000000..e634edef --- /dev/null +++ b/packages/dsh-desktop-market-installer/remove-tree.mjs @@ -0,0 +1,71 @@ +/** + * Delete a directory tree without `rm`'s recursive mode. + * + * Node's recursive `rm` is unusable on this desktop's Windows installs: for + * any path containing a non-ASCII character it removes nothing and still + * resolves successfully. A profile lives under the user's home, so a single + * non-ASCII character in the account name — `C:\Users\数据项素\…` — silently + * disables every cleanup performed there, and each one reports the removal it + * did not make: + * + * rmSync('C:\\Users\\Public\\ascii', { recursive: true }) -> gone + * rmSync('C:\\Users\\Public\\中文', { recursive: true }) -> still there + * + * The single-entry calls are unaffected, so the walk below is done by hand: + * `unlink` each file, `rmdir` each directory on the way out. Symlinks are + * unlinked rather than followed, so a link into pnpm's store never takes the + * store's contents with it. + * + * This is the Harness-side copy of `src/main/state/remove-tree.ts`. The two + * run in different processes — this one inside Harness, the other in the + * desktop's main process — and the desktop's build does not reach into this + * package's sources, so the walk is stated in both places rather than shared + * through an import that neither side could satisfy. + */ +import { lstat, readdir, rmdir, unlink } from 'node:fs/promises' +import { join } from 'node:path' + +/** + * Remove a file or directory tree. + * @param {string} path - the path to remove; a missing path is not an error. + */ +export async function removeTree(path) { + let entry + try { + entry = await lstat(path) + } catch (error) { + if (error?.code === 'ENOENT') return + throw error + } + + // A symlink to a directory reports as one; unlinking is what detaches it + // without touching what it points at. + if (!entry.isDirectory() || entry.isSymbolicLink()) { + await unlink(path) + return + } + + for (const child of await readdir(path, { withFileTypes: true })) { + await removeTree(join(path, child.name)) + } + await rmdir(path) +} + +/** + * Remove a tree, reporting whether it is gone rather than throwing. + * @param {string} path - the path to remove. + * @returns {Promise} whether the path is no longer present. + */ +export async function removeTreeIfPossible(path) { + try { + await removeTree(path) + } catch { + // Fall through to the check: a partial removal still counts for nothing. + } + try { + await lstat(path) + return false + } catch (error) { + return error?.code === 'ENOENT' + } +} diff --git a/src/main/index.ts b/src/main/index.ts index 7832c5c6..b52efc60 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -16,7 +16,11 @@ import { } from 'electron' import { extractFailureCause, HarnessRuntime } from './runtime/harness-runtime' import { launchDisclaimedUtilityProcess } from './runtime/disclaimed-utility-process' -import { removeProfilePluginWithDsh } from './runtime/profile-plugin-command' +import { + installProfileDependenciesWithDsh, + removeProfilePluginWithDsh +} from './runtime/profile-plugin-command' +import { clearDamagedPackageDirectories, hasProfile } from './state/profile-repair' import { LanMobileBridge } from './mobile/lan-mobile-bridge' import { detectPluginRecovery, @@ -261,6 +265,21 @@ function bundledNodePath(): string { return join(app.getAppPath(), 'node_modules', 'node', 'bin', executable) } +/** + * The packaged lock-recovery runner. The Harness-side installer stages its own + * copy into .desktop-bin; the desktop writes shims to that same directory, so + * it points at the same runner rather than replacing them with a plain pnpm + * call that would silently drop the recovery. + */ +function bundledPnpmRunnerPath(): string { + return join( + app.getAppPath(), + 'node_modules', + 'dsh-desktop-market-installer', + 'pnpm-runner.mjs' + ) +} + function bundledPnpmEntryPath(): string { const root = join(app.getAppPath(), 'node_modules', 'pnpm', 'bin') const candidates = [join(root, 'pnpm.cjs'), join(root, 'pnpm.mjs')] @@ -445,13 +464,56 @@ async function showSplash(): Promise { window.focus() } +/** + * Clear what an earlier failed package operation left behind, then put the + * packages back — both while Harness is stopped, the only moment either is + * safe. A profile damaged by an older build heals on the first launch of this + * one; an undamaged profile costs a directory scan. Failure here is not fatal: + * the prune below still keeps the profile bootable, and Harness reports + * whatever remains. + */ +async function repairProfilePackages(dshHome: string): Promise { + try { + if (!hasProfile(dshHome)) return + const removed = await clearDamagedPackageDirectories(dshHome) + if (removed.length === 0) return + + runtime.note( + `[desktop] repairing profile: cleared ${removed.length} damaged package ${ + removed.length === 1 ? 'directory' : 'directories' + }` + ) + const result = await installProfileDependenciesWithDsh({ + dshHome, + dshEntryPath: dshEntryPath(), + nodeExecutablePath: bundledNodePath(), + pnpmEntryPath: bundledPnpmEntryPath(), + pnpmRunnerPath: bundledPnpmRunnerPath() + }) + runtime.note( + result.ok + ? '[desktop] profile repair completed' + : `[desktop] profile repair failed: ${result.detail ?? 'unknown error'}` + ) + } catch (error) { + runtime.note( + `[desktop] profile repair failed: ${error instanceof Error ? error.message : String(error)}` + ) + } +} + function launchHarness(): Promise { if (harnessLaunchOperation) return harnessLaunchOperation harnessLaunchOperation = (async () => { const dshHome = join(app.getPath('userData'), 'harness') - await pruneMissingProfileBundles(dshHome).catch(() => false) await showSplash() + // The repair only holds on a stopped Harness, and a restart still has the + // previous one running: start() stops it, but that is after the repair. + // Stopping here is what makes the window this launch path assumes. + await runtime.stop() + await repairProfilePackages(dshHome) + await pruneMissingProfileBundles(dshHome).catch(() => false) await runtime.start(launchDirectory) })().finally(() => { harnessLaunchOperation = undefined @@ -705,6 +767,7 @@ async function showPluginRecovery(options?: { dshEntryPath: dshEntryPath(), nodeExecutablePath: bundledNodePath(), pnpmEntryPath: bundledPnpmEntryPath(), + pnpmRunnerPath: bundledPnpmRunnerPath(), environment: process.env }, pluginName diff --git a/src/main/runtime/harness-runtime.ts b/src/main/runtime/harness-runtime.ts index 49607777..9a7647ec 100644 --- a/src/main/runtime/harness-runtime.ts +++ b/src/main/runtime/harness-runtime.ts @@ -1,6 +1,6 @@ import type { SpawnOptionsWithoutStdio } from 'node:child_process' import type { EventEmitter } from 'node:events' -import { createWriteStream, existsSync, type WriteStream } from 'node:fs' +import { createWriteStream, existsSync, mkdirSync, type WriteStream } from 'node:fs' import { mkdir } from 'node:fs/promises' import { createServer } from 'node:net' import { dirname, join } from 'node:path' @@ -52,6 +52,10 @@ export function buildHarnessSpawnOptions( const { ELECTRON_RUN_AS_NODE: _runAsNode, ...parentEnvironment } = environment const pathKey = platform === 'win32' ? 'Path' : 'PATH' + // ELECTRON_RUN_AS_NODE must not reach the Harness process itself: the macOS + // utility process is launched with Chromium switches (--type=utility, …) + // that Node rejects as bad options. The Harness entry re-declares Node mode + // from the inside, for its children only. return { cwd: launchDirectory, env: { @@ -141,7 +145,7 @@ export class HarnessRuntime { await mkdir(this.options.dshHome, { recursive: true }) await mkdir(dirname(this.options.logPath), { recursive: true }) - this.logStream = createWriteStream(this.options.logPath, { flags: 'a' }) + this.logStream ??= createWriteStream(this.options.logPath, { flags: 'a' }) const port = await reservePort() const url = `http://127.0.0.1:${port}` @@ -264,6 +268,23 @@ ${cause}` } } + /** + * Record a line the desktop wants in the Harness log, including before a + * launch: what happens to the profile between launches is exactly what + * someone reading the log after a failed install needs to see. + */ + note(line: string): void { + if (!this.logStream) { + try { + mkdirSync(dirname(this.options.logPath), { recursive: true }) + this.logStream = createWriteStream(this.options.logPath, { flags: 'a' }) + } catch { + // Keep the line in the in-memory buffer regardless. + } + } + this.writeLog(line) + } + private writeLog(line: string): void { this.logLines.push(line) if (this.logLines.length > 200) this.logLines.splice(0, this.logLines.length - 200) diff --git a/src/main/runtime/profile-plugin-command.ts b/src/main/runtime/profile-plugin-command.ts index 3bbbc1c7..2a7e7eb7 100644 --- a/src/main/runtime/profile-plugin-command.ts +++ b/src/main/runtime/profile-plugin-command.ts @@ -5,6 +5,7 @@ import { delimiter, dirname, join } from 'node:path' const PROFILE = 'web' const OPERATION_TIMEOUT_MS = 15 * 60 * 1000 +const REPAIR_TIMEOUT_MS = 5 * 60 * 1000 const MAX_OUTPUT_BYTES = 32 * 1024 export interface ProfilePluginCommandOptions { @@ -12,6 +13,13 @@ export interface ProfilePluginCommandOptions { dshEntryPath: string nodeExecutablePath: string pnpmEntryPath: string + /** + * The packaged lock-recovery runner. The shims below share a directory with + * the ones the Harness-side installer writes, so leaving this out would + * replace a runner-routed pnpm with a plain one and silently drop the + * recovery until Harness next rewrote them. + */ + pnpmRunnerPath?: string environment?: NodeJS.ProcessEnv } @@ -31,14 +39,44 @@ export function buildProfilePluginRemoveArguments( return [dshEntryPath, 'plugin', '--profile', PROFILE, 'remove', pluginName] } +/** + * Reinstall everything the profile manifest asks for. This runs before Harness + * starts, which is the only moment the packages it would otherwise hold open + * can be replaced — so it is also how a profile left damaged by an earlier + * failure gets its packages back. + * + * The lockfile is explicitly allowed to move. This repair runs with CI set, + * which is pnpm's signal to install with a frozen lockfile, and the profile it + * has to repair is exactly the one where the lockfile cannot be trusted: a + * `pnpm add` that fails while linking has already written the new version into + * pnpm-lock.yaml while package.json still names the old one. Frozen there + * fails on the divergence — `ERR_PNPM_OUTDATED_LOCKFILE` — which turns the one + * path out of a damaged profile into another way to stay in it. The manifest + * is what the profile is meant to be; the lockfile follows it. + */ +export function buildProfileInstallArguments(dshEntryPath: string): string[] { + return [dshEntryPath, 'plugin', '--profile', PROFILE, 'install', '--no-frozen-lockfile'] +} + +export function buildPnpmShimCommand(options: ProfilePluginCommandOptions): string[] { + const runner = + options.pnpmRunnerPath !== undefined && existsSync(options.pnpmRunnerPath) + ? [options.pnpmRunnerPath] + : [] + return [...runner, options.pnpmEntryPath] +} + export async function ensureProfilePnpmShim(options: ProfilePluginCommandOptions): Promise { const directory = join(options.dshHome, '.desktop-bin') await mkdir(directory, { recursive: true }) + const command = buildPnpmShimCommand(options) if (process.platform === 'win32') { await writeFile( join(directory, 'pnpm.cmd'), - `@chcp 65001 >nul\r\n@echo off\r\n"${options.nodeExecutablePath}" "${options.pnpmEntryPath}" %*\r\n`, + `@chcp 65001 >nul\r\n@echo off\r\n"${options.nodeExecutablePath}" ${command + .map((part) => `"${part}"`) + .join(' ')} %*\r\n`, 'utf8' ) await writeFile( @@ -50,7 +88,9 @@ export async function ensureProfilePnpmShim(options: ProfilePluginCommandOptions const pnpmPath = join(directory, 'pnpm') await writeFile( pnpmPath, - `#!/bin/sh\nexec ${shellQuote(options.nodeExecutablePath)} ${shellQuote(options.pnpmEntryPath)} "$@"\n`, + `#!/bin/sh\nexec ${shellQuote(options.nodeExecutablePath)} ${command + .map(shellQuote) + .join(' ')} "$@"\n`, { encoding: 'utf8', mode: 0o755 } ) await chmod(pnpmPath, 0o755) @@ -92,6 +132,23 @@ export function buildProfilePluginCommandEnvironment( return result } +/** + * The line worth reporting from a failed run. dsh's own wrapper ("pnpm failed + * in profile directory …") is always last and names no cause, so a line that + * does name one wins — otherwise a failure reads as a dead end. + */ +export function diagnosticLine(output: string): string | undefined { + const lines = output + .trim() + .split(/\r?\n/u) + .map((line) => line.trim()) + .filter(Boolean) + const named = lines.filter((line: string) => + /EPERM|EBUSY|EACCES|EEXIST|ENOTEMPTY|ENOENT|ERR_PNPM|error:/u.test(line) + ) + return (named.at(-1) ?? lines.at(-1))?.slice(0, 800) +} + function killProcessTree(child: ReturnType): void { if (child.exitCode !== null || !child.pid) return if (process.platform === 'win32') { @@ -111,6 +168,28 @@ function killProcessTree(child: ReturnType): void { export async function removeProfilePluginWithDsh( options: ProfilePluginCommandOptions, pluginName: string +): Promise { + return runProfileCommand(options, buildProfilePluginRemoveArguments(options.dshEntryPath, pluginName), 'Plugin removal', OPERATION_TIMEOUT_MS) +} + +/** + * Restore the profile's packages with Harness stopped. + * @param timeoutMs - shorter than a user-initiated operation on purpose: this + * one sits between the user and their window, so it gives up rather than + * turning a damaged profile into a launch that looks hung. + */ +export async function installProfileDependenciesWithDsh( + options: ProfilePluginCommandOptions, + timeoutMs = REPAIR_TIMEOUT_MS +): Promise { + return runProfileCommand(options, buildProfileInstallArguments(options.dshEntryPath), 'Profile repair', timeoutMs) +} + +async function runProfileCommand( + options: ProfilePluginCommandOptions, + commandArguments: string[], + label: string, + timeoutMs: number ): Promise { const requiredPaths = [ options.dshEntryPath, @@ -137,7 +216,7 @@ export async function removeProfilePluginWithDsh( const child = spawn( options.nodeExecutablePath, - buildProfilePluginRemoveArguments(options.dshEntryPath, pluginName), + commandArguments, { cwd: profileDirectory, env: environment, @@ -158,7 +237,7 @@ export async function removeProfilePluginWithDsh( const timer = setTimeout(() => { timedOut = true killProcessTree(child) - }, OPERATION_TIMEOUT_MS) + }, timeoutMs) try { const exit = await new Promise<{ code: number | null; signal: NodeJS.Signals | null }>( @@ -168,15 +247,18 @@ export async function removeProfilePluginWithDsh( } ) if (timedOut) { - return { ok: false, detail: 'Plugin removal timed out after 15 minutes.' } + return { + ok: false, + detail: `${label} timed out after ${Math.round(timeoutMs / 60_000)} minutes.` + } } if (exit.code !== 0) { - const detail = output.trim().split(/\r?\n/u).at(-1)?.slice(0, 800) + const detail = diagnosticLine(output) return { ok: false, detail: detail || - `Plugin removal exited with ${exit.signal ? `signal ${exit.signal}` : `code ${exit.code}`}.` + `${label} exited with ${exit.signal ? `signal ${exit.signal}` : `code ${exit.code}`}.` } } return { ok: true } diff --git a/src/main/state/plugin-recovery.ts b/src/main/state/plugin-recovery.ts index fad72676..2f824df9 100644 --- a/src/main/state/plugin-recovery.ts +++ b/src/main/state/plugin-recovery.ts @@ -2,6 +2,18 @@ import { existsSync } from 'node:fs' import { readFile, readdir, rm, writeFile } from 'node:fs/promises' import { dirname, join, resolve } from 'node:path' import { parse } from 'yaml' +import { removeTree } from './remove-tree' + +/** + * Directories under the profile's node_modules that no longer belong to any + * package: pnpm's `_tmp__` staging left by an interrupted run, + * and the `.dsh-old-` copies the packaged pnpm runner moves aside + * when Windows refuses to replace a directory still held open. Both are only + * safely removable before Harness starts, which is when this sweep runs. + */ +export function isDisposableModuleDirectory(name: string): boolean { + return name.includes('_tmp_') || name.includes('.dsh-old-') +} export function profilePackageJsonPath(dshHome: string): string { return join(dshHome, 'profiles', 'web', 'package.json') @@ -450,7 +462,7 @@ export async function resetPluginProfile( if (existsSync(nodeModulesPath)) { if (failingPlugin) { const pluginDir = join(nodeModulesPath, failingPlugin) - await rm(pluginDir, { recursive: true, force: true }).catch(() => undefined) + await removeTree(pluginDir).catch(() => undefined) if (failingPlugin.startsWith('@')) { const scope = failingPlugin.split('/')[0] if (scope) { @@ -458,7 +470,7 @@ export async function resetPluginProfile( try { const files = await readdir(scopeDir) if (files.length === 0) { - await rm(scopeDir, { recursive: true, force: true }).catch(() => undefined) + await removeTree(scopeDir).catch(() => undefined) } } catch {} } @@ -495,8 +507,8 @@ export async function pruneMissingProfileBundles(dshHome: string): Promise undefined) + if (entry.isDirectory() && isDisposableModuleDirectory(entry.name)) { + await removeTree(join(nodeModulesPath, entry.name)).catch(() => undefined) } } } catch { diff --git a/src/main/state/profile-repair.ts b/src/main/state/profile-repair.ts new file mode 100644 index 00000000..4b5b6c3b --- /dev/null +++ b/src/main/state/profile-repair.ts @@ -0,0 +1,128 @@ +import { existsSync } from 'node:fs' +import { lstat, readFile, readdir } from 'node:fs/promises' +import { dirname, join } from 'node:path' +import { isDisposableModuleDirectory, profilePackageJsonPath } from './plugin-recovery' +import { removeTree } from './remove-tree' + +/** + * What a failed package operation leaves behind, and how the next launch gets + * rid of it. + * + * A pnpm run that dies partway through — the Windows locked rename is the one + * that keeps happening — leaves a directory under the profile's node_modules + * that carries a package's name without being one. pnpm cannot rename its + * staging directory onto that name afterwards, so every later attempt fails + * the same way: the profile is stuck until someone deletes the leftover by + * hand. + * + * Nothing holds these directories before Harness starts, which makes the + * launch path the one place they can be cleared safely. Clearing alone would + * amputate — the packages are still wanted — so a damaged profile is followed + * by an install that puts them back, and only what the install could not + * restore is pruned from the manifest afterwards. + */ + +/** Entries pnpm owns that are not packages and must never be judged as ones. */ +function isReservedEntry(name: string): boolean { + return name.startsWith('.') +} + +async function isMaterializedPackage(directory: string): Promise { + try { + const manifest = await readFile(join(directory, 'package.json'), 'utf8') + return typeof (JSON.parse(manifest) as { name?: unknown }).name === 'string' + } catch { + return false + } +} + +/** + * Package directories that carry a name without being a package: no readable + * manifest behind it. Scoped directories are inspected one level down, where + * the packages actually live. Symlinks are left alone — pnpm's isolated layout + * points them into the virtual store, and a broken link is not ours to judge. + * + * A package that is intact still gets its own node_modules looked through, + * because that is where the leftovers hide once a dependency of a dependency + * is the one being replaced: `cytoscape-fcose/node_modules/cose-base.dsh-old-…` + * is invisible to a scan that stops at the top level, and it accumulates one + * copy per attempt. + * @param dshHome - the desktop's DSH home. + * @returns absolute paths, in the order found. + */ +export async function findDamagedPackageDirectories(dshHome: string): Promise { + const nodeModulesPath = join(dirname(profilePackageJsonPath(dshHome)), 'node_modules') + const damaged: string[] = [] + + const scan = async (directory: string, allowScopes: boolean): Promise => { + let entries + try { + entries = await readdir(directory, { withFileTypes: true }) + } catch { + return + } + + for (const entry of entries) { + if (isReservedEntry(entry.name) || entry.isSymbolicLink() || !entry.isDirectory()) continue + const path = join(directory, entry.name) + + if (isDisposableModuleDirectory(entry.name)) { + damaged.push(path) + continue + } + if (allowScopes && entry.name.startsWith('@')) { + await scan(path, false) + continue + } + if (!(await isMaterializedPackage(path))) { + damaged.push(path) + continue + } + await scan(join(path, 'node_modules'), true) + } + } + + await scan(nodeModulesPath, true) + return damaged +} + +/** + * Clear what {@link findDamagedPackageDirectories} found. + * + * Every removal is confirmed rather than assumed. Node's recursive `rm` + * reports success without removing anything under a non-ASCII path — see + * {@link removeTree} — and a sweep that trusted it announced twelve cleared + * directories over a profile where all twelve were still on disk, which made + * the log the least reliable account of the profile's state. + * @returns the paths actually removed. + */ +export async function clearDamagedPackageDirectories(dshHome: string): Promise { + const damaged = await findDamagedPackageDirectories(dshHome) + const removed: string[] = [] + + for (const path of damaged) { + try { + await removeTree(path) + if (!(await exists(path))) removed.push(path) + } catch { + // Still held by something. The install below will fail on this name and + // report it, which beats deleting half of it here. + } + } + + return removed +} + +async function exists(path: string): Promise { + try { + await lstat(path) + return true + } catch { + return false + } +} + +/** Whether the profile is worth repairing at all — no profile, nothing to do. */ +export function hasProfile(dshHome: string): boolean { + return existsSync(profilePackageJsonPath(dshHome)) +} diff --git a/src/main/state/remove-tree.ts b/src/main/state/remove-tree.ts new file mode 100644 index 00000000..92ce4e7d --- /dev/null +++ b/src/main/state/remove-tree.ts @@ -0,0 +1,78 @@ +import { lstat, readdir, rm, rmdir, unlink } from 'node:fs/promises' +import { join } from 'node:path' + +/** + * Delete a directory tree without `rm`'s recursive mode. + * + * Node's recursive `rm` is unusable on this desktop's Windows installs: for + * any path containing a non-ASCII character it removes nothing and still + * resolves successfully. A profile lives under the user's home, so a single + * non-ASCII character in the account name — `C:\Users\数据项素\…` — silently + * disables every cleanup the desktop performs, and each one reports the + * removal it did not make: + * + * rmSync('C:\\Users\\Public\\ascii', { recursive: true }) -> gone + * rmSync('C:\\Users\\Public\\中文', { recursive: true }) -> still there + * + * The single-entry calls are unaffected, so the walk below is done by hand: + * `unlink` each file, `rmdir` each directory on the way out. Symlinks are + * unlinked rather than followed, so a link into pnpm's store never takes the + * store's contents with it. + * + * pnpm's own cleanup is the same broken call (`@zkochan/rimraf` is a wrapper + * around `fs.rmSync(p, { recursive: true, force: true })`), which is why a + * package that has to be replaced on these installs can never be: pnpm's + * recovery from a blocked rename deletes the destination first, that delete + * does nothing, and the retry it guards then fails for the full minute it + * allows. Nothing here can fix pnpm, but everything the desktop clears for it + * has to actually clear. + * + * @param path - the file or directory to remove; a missing path is not an error. + */ +export async function removeTree(path: string): Promise { + let entry + try { + entry = await lstat(path) + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'ENOENT') return + throw error + } + + // A symlink to a directory reports as one; unlinking is what detaches it + // without touching what it points at. + if (!entry.isDirectory() || entry.isSymbolicLink()) { + await unlink(path) + return + } + + const children = await readdir(path, { withFileTypes: true }) + for (const child of children) { + await removeTree(join(path, child.name)) + } + await rmdir(path) +} + +/** + * Remove a tree, reporting whether it is gone rather than throwing. Useful + * where a leftover that cannot be removed is worth reporting but not worth + * failing over — the caller's next step usually names it anyway. + * @param path - the file or directory to remove. + * @returns whether the path is no longer present. + */ +export async function removeTreeIfPossible(path: string): Promise { + try { + await removeTree(path) + return true + } catch { + // A last try through the platform's own recursion: it is a no-op on the + // paths this module exists for, but it costs nothing and covers whatever + // the hand-walk could not express. + try { + await rm(path, { recursive: true, force: true }) + await lstat(path) + return false + } catch (error) { + return (error as NodeJS.ErrnoException).code === 'ENOENT' + } + } +} diff --git a/test/market-installer.test.js b/test/market-installer.test.js index ab4ebcc7..82f9f2d3 100644 --- a/test/market-installer.test.js +++ b/test/market-installer.test.js @@ -14,7 +14,8 @@ import { ensurePnpmShim, isTrustedRequest, readMarketInstallation, - resolvePnpmEntry + resolvePnpmEntry, + stagePnpmRunner } from '../packages/dsh-desktop-market-installer/index.js' describe('desktop plugin market installer', () => { @@ -26,10 +27,10 @@ describe('desktop plugin market installer', () => { 'web', 'add', '--save-exact', - 'dshmarket@1.9.0' + 'dshmarket@1.15.0' ]) expect(MARKET_PACKAGE).toBe('dshmarket') - expect(RECOMMENDED_MARKET_VERSION).toBe('1.9.0') + expect(RECOMMENDED_MARKET_VERSION).toBe('1.15.0') expect(STATUS_PATH).toBe('/dsh-desktop/market-installer/status') expect(INSTALL_PATH).toBe('/dsh-desktop/market-installer/install') expect(UNINSTALL_PATH).toBe('/dsh-desktop/market-installer/uninstall') @@ -52,18 +53,33 @@ describe('desktop plugin market installer', () => { const binDir = await ensurePnpmShim(home) expect(binDir).toBe(join(home, '.desktop-bin')) + const runner = await readFile(join(binDir, 'pnpm-runner.mjs'), 'utf8') + expect(runner).toContain('runWithLockRecovery') + await expect(stagePnpmRunner(binDir)).resolves.toBe(join(binDir, 'pnpm-runner.mjs')) + // A directory that cannot hold the staged copy still leaves pnpm reachable + // through the packaged original rather than taking the shims down. + await expect(stagePnpmRunner(join(binDir, 'missing', 'deeper'))).resolves.toMatch( + /packages[/\\]dsh-desktop-market-installer[/\\]pnpm-runner\.mjs$/u + ) + if (process.platform === 'win32') { const pnpmCmd = await readFile(join(binDir, 'pnpm.cmd'), 'utf8') const nodeCmd = await readFile(join(binDir, 'node.cmd'), 'utf8') expect(pnpmCmd).toContain(process.execPath) expect(pnpmCmd).toContain('pnpm') + expect(pnpmCmd).toContain(join(binDir, 'pnpm-runner.mjs')) + expect(pnpmCmd).toContain('set ELECTRON_RUN_AS_NODE=1') expect(nodeCmd).toContain(process.execPath) + expect(nodeCmd).toContain('set ELECTRON_RUN_AS_NODE=1') } else { const pnpmScript = await readFile(join(binDir, 'pnpm'), 'utf8') const nodeScript = await readFile(join(binDir, 'node'), 'utf8') expect(pnpmScript).toContain(process.execPath) expect(pnpmScript).toContain('pnpm') + expect(pnpmScript).toContain(join(binDir, 'pnpm-runner.mjs')) + expect(pnpmScript).toContain('export ELECTRON_RUN_AS_NODE=1') expect(nodeScript).toContain(process.execPath) + expect(nodeScript).toContain('export ELECTRON_RUN_AS_NODE=1') } const npmrc = await readFile(join(home, 'profiles', 'web', '.npmrc'), 'utf8') @@ -71,21 +87,63 @@ describe('desktop plugin market installer', () => { expect(npmrc).toContain('child-concurrency=1') }) - it('cleans up stale _tmp_ directories left by interrupted installations', async () => { + it('cleans up staging and sidelined directories left by interrupted installations', async () => { const home = await mkdtemp(join(tmpdir(), 'dsh-market-clean-')) const nodeModules = join(home, 'profiles', 'web', 'node_modules') const staleTmpDir = join(nodeModules, 'argparse_tmp_12345_1') + const sidelinedDir = join(nodeModules, 'argparse.dsh-old-1787317710932') const validDir = join(nodeModules, 'argparse') await mkdir(staleTmpDir, { recursive: true }) + await mkdir(sidelinedDir, { recursive: true }) await mkdir(validDir, { recursive: true }) await cleanStaleTemporaryDirectories(home) const { existsSync } = await import('node:fs') expect(existsSync(staleTmpDir)).toBe(false) + expect(existsSync(sidelinedDir)).toBe(false) expect(existsSync(validDir)).toBe(true) }) + it('cleans the leftovers inside a package’s own node_modules', async () => { + // A replaced dependency of a dependency stages under the dependent, so a + // sweep that stops at the top level leaves one copy behind per attempt. + const home = await mkdtemp(join(tmpdir(), 'dsh-market-clean-nested-')) + const nodeModules = join(home, 'profiles', 'web', 'node_modules') + const nested = join(nodeModules, 'cytoscape-fcose', 'node_modules') + const scoped = join(nodeModules, '@deepseek-ai') + const nestedStale = join(nested, 'cose-base.dsh-old-1787327060846') + const scopedStale = join(scoped, 'dsh-settings_tmp_7408_2') + const kept = join(nested, 'cose-base') + await mkdir(nestedStale, { recursive: true }) + await mkdir(scopedStale, { recursive: true }) + await mkdir(kept, { recursive: true }) + await writeFile(join(kept, 'package.json'), JSON.stringify({ name: 'cose-base' }), 'utf8') + + await cleanStaleTemporaryDirectories(home) + + const { existsSync } = await import('node:fs') + expect(existsSync(nestedStale)).toBe(false) + expect(existsSync(scopedStale)).toBe(false) + expect(existsSync(kept)).toBe(true) + }) + + it('clears a tree whose path is not ASCII', async () => { + // Node's recursive `rm` reports success and removes nothing under such a + // path on Windows. A profile lives under the user's home, so one non-ASCII + // character in the account name used to disable this sweep entirely. + const home = join(await mkdtemp(join(tmpdir(), 'dsh-market-unicode-')), '数据项素') + const nodeModules = join(home, 'profiles', 'web', 'node_modules') + const stale = join(nodeModules, 'dshmarket_tmp_7408_13', 'lib') + await mkdir(stale, { recursive: true }) + await writeFile(join(stale, 'index.js'), 'export default 1', 'utf8') + + await cleanStaleTemporaryDirectories(home) + + const { existsSync } = await import('node:fs') + expect(existsSync(join(nodeModules, 'dshmarket_tmp_7408_13'))).toBe(false) + }) + it('reports both the requested dependency and installed package version', async () => { const home = await mkdtemp(join(tmpdir(), 'dsh-market-status-')) const profile = join(home, 'profiles', 'web') diff --git a/test/plugin-recovery.test.ts b/test/plugin-recovery.test.ts index f7c67673..ce6cc7f9 100644 --- a/test/plugin-recovery.test.ts +++ b/test/plugin-recovery.test.ts @@ -3,6 +3,7 @@ import { join } from 'node:path' import { afterEach, beforeEach, describe, expect, it } from 'vitest' import { parse, stringify } from 'yaml' import { + isDisposableModuleDirectory, isThirdPartyPackageName, profilePackageJsonPath, pruneMissingProfileBundles, @@ -697,5 +698,16 @@ describe('plugin-recovery', () => { const modified = await pruneMissingProfileBundles(testDir) expect(modified).toBe(false) }) + + it('sweeps pnpm staging and sidelined package directories before launch', () => { + // Windows refuses to replace a directory something still holds open, so + // the packaged pnpm runner moves the blocked package aside and lets pnpm + // install over the freed name. Nothing holds either leftover before + // Harness starts, which is when this sweep runs. + expect(isDisposableModuleDirectory('argparse_tmp_19856_4')).toBe(true) + expect(isDisposableModuleDirectory('argparse.dsh-old-1787317710932')).toBe(true) + expect(isDisposableModuleDirectory('argparse')).toBe(false) + expect(isDisposableModuleDirectory('js-yaml')).toBe(false) + }) }) diff --git a/test/pnpm-runner.test.js b/test/pnpm-runner.test.js new file mode 100644 index 00000000..f49ec11d --- /dev/null +++ b/test/pnpm-runner.test.js @@ -0,0 +1,377 @@ +import { EventEmitter } from 'node:events' +import { join } from 'node:path' +import { describe, expect, it, vi } from 'vitest' +import { + MARKER, + SIDELINE_MARKER, + blockedTargets, + lockedRenameTarget, + runWithLockRecovery, + sidelinePath +} from '../packages/dsh-desktop-market-installer/pnpm-runner.mjs' + +const WINDOWS_LOCK_FAILURE = [ + 'Update failed: dshmarket', + "error: EPERM: operation not permitted, rename 'C:\\Users\\u\\AppData\\Roaming\\dsh-desktop-dev\\harness\\profiles\\web\\node_modules\\argparse_tmp_19856_4' -> 'C:\\Users\\u\\AppData\\Roaming\\dsh-desktop-dev\\harness\\profiles\\web\\node_modules\\argparse'", + ' at Worker. (D:\\AA\\DSH Desktop Dev\\resources\\app\\node_modules\\pnpm\\dist\\pnpm.cjs:104217:22)' +].join('\n') + +const BLOCKED_TARGET = + 'C:\\Users\\u\\AppData\\Roaming\\dsh-desktop-dev\\harness\\profiles\\web\\node_modules\\argparse' + +function fakePnpm(runs) { + const calls = [] + const spawnProcess = (executable, args) => { + const run = runs[calls.length] ?? { code: 0, output: '' } + calls.push({ executable, args }) + const child = new EventEmitter() + child.stdout = new EventEmitter() + child.stderr = new EventEmitter() + child.pid = 4242 + child.kill = () => { + child.emit('exit', null, 'SIGKILL') + return true + } + if (run.silent) return child + queueMicrotask(() => { + if (run.output) child.stderr.emit('data', run.output) + child.emit('exit', run.code, null) + }) + return child + } + return { spawnProcess, calls } +} + +describe('packaged pnpm runner', () => { + it('recognizes a Windows locked rename inside a profile', () => { + expect(lockedRenameTarget(WINDOWS_LOCK_FAILURE)).toBe(BLOCKED_TARGET) + expect( + lockedRenameTarget( + "EBUSY: resource busy or locked, rename '/p/node_modules/a_tmp_1_1' -> '/p/node_modules/a'" + ) + ).toBe('/p/node_modules/a') + }) + + it('leaves unrelated failures alone', () => { + expect(lockedRenameTarget('ERR_PNPM_FETCH_500 GET https://registry/x failed')).toBeUndefined() + expect( + lockedRenameTarget("EPERM: operation not permitted, rename '/tmp/a' -> '/etc/passwd'") + ).toBeUndefined() + expect(lockedRenameTarget('')).toBeUndefined() + }) + + it('names the sidelined directory next to the blocked one', () => { + expect(sidelinePath('/p/node_modules/argparse', 42)).toBe( + `/p/node_modules/argparse${SIDELINE_MARKER}42` + ) + }) + + it('passes a successful run straight through', async () => { + const { spawnProcess, calls } = fakePnpm([{ code: 0, output: '' }]) + const moveAside = vi.fn() + + const result = await runWithLockRecovery('/node', ['/pnpm.cjs', 'add', 'x'], { + spawnProcess, + moveAside, + wait: async () => undefined + }) + + expect(result.code).toBe(0) + expect(calls).toHaveLength(1) + expect(moveAside).not.toHaveBeenCalled() + }) + + it('does not retry a failure that is not a locked rename', async () => { + const { spawnProcess, calls } = fakePnpm([{ code: 1, output: 'ERR_PNPM_NO_MATCHING_VERSION' }]) + + const result = await runWithLockRecovery('/node', ['/pnpm.cjs', 'add', 'x'], { + spawnProcess, + wait: async () => undefined + }) + + expect(result.code).toBe(1) + expect(calls).toHaveLength(1) + }) + + it('retries once for a lock that clears on its own', async () => { + const { spawnProcess, calls } = fakePnpm([ + { code: 1, output: WINDOWS_LOCK_FAILURE }, + { code: 0, output: '' } + ]) + const moveAside = vi.fn() + + const result = await runWithLockRecovery('/node', ['/pnpm.cjs', 'add', 'x'], { + spawnProcess, + moveAside, + wait: async () => undefined + }) + + expect(result.code).toBe(0) + expect(calls).toHaveLength(2) + expect(moveAside).not.toHaveBeenCalled() + }) + + it('moves the held directory aside and installs over the freed name', async () => { + const { spawnProcess, calls } = fakePnpm([ + { code: 1, output: WINDOWS_LOCK_FAILURE }, + { code: 1, output: WINDOWS_LOCK_FAILURE }, + { code: 0, output: '' } + ]) + const moveAside = vi.fn(async () => undefined) + + const result = await runWithLockRecovery('/node', ['/pnpm.cjs', 'add', 'x'], { + spawnProcess, + moveAside, + exists: () => true, + listEntries: async () => [], + wait: async () => undefined, + now: () => 1234 + }) + + expect(result.code).toBe(0) + expect(calls).toHaveLength(3) + expect(moveAside).toHaveBeenCalledWith( + BLOCKED_TARGET, + `${BLOCKED_TARGET}${SIDELINE_MARKER}1234` + ) + }) + + it('stops a pnpm that has gone silent instead of waiting out the host timeout', async () => { + const { spawnProcess, calls } = fakePnpm([{ silent: true }]) + const lines = [] + + const result = await runWithLockRecovery('/node', ['/pnpm.cjs', 'add', 'x'], { + spawnProcess, + idleTimeoutMs: 5, + kill: (child) => child.kill(), + watchActivity: () => () => undefined, + report: (message) => lines.push(message) + }) + + expect(result.code).toBe(1) + expect(result.idleTimedOut).toBe(true) + // A wedged run is not retried — three stuck runs are three times the wait. + expect(calls).toHaveLength(1) + expect(lines[0]).toContain('touched nothing') + }) + + it('keeps a quiet run that is still writing packages', async () => { + // Without a TTY pnpm drops its progress display: resolution, a cold + // download and a large link phase can each pass without a line. Killing + // those would turn a slow install into a failed one. + const { spawnProcess } = fakePnpm([{ silent: true }]) + let stopWatching = 0 + let signalActivity = () => undefined + + const running = runWithLockRecovery('/node', ['/pnpm.cjs', 'add', 'x'], { + spawnProcess, + idleTimeoutMs: 40, + kill: (child) => child.kill(), + watchActivity: (onActivity) => { + signalActivity = onActivity + return () => { + stopWatching += 1 + } + }, + report: () => undefined + }) + + // Three quiet-but-busy stretches, each shorter than the allowance. + for (let beat = 0; beat < 3; beat += 1) { + await new Promise((resolve) => setTimeout(resolve, 25)) + signalActivity() + } + signalActivity() + const result = await Promise.race([ + running, + new Promise((resolve) => setTimeout(() => resolve('still running'), 20)) + ]) + + expect(result).toBe('still running') + signalActivity = () => undefined + await expect(running).resolves.toMatchObject({ idleTimedOut: true }) + expect(stopWatching).toBe(1) + }) + + it('recovers a run that named its blocker and then hung', async () => { + // pnpm raises the locked rename inside a worker and has been seen never to + // unwind from it. That run has to be stopped — and it is also the exact + // run this recovery exists for, so being stopped must not skip it. + const calls = [] + const spawnProcess = () => { + const attempt = calls.length + calls.push(attempt) + const child = new EventEmitter() + child.stdout = new EventEmitter() + child.stderr = new EventEmitter() + child.pid = 4242 + child.kill = () => { + child.emit('exit', null, 'SIGKILL') + return true + } + queueMicrotask(() => { + if (attempt < 2) { + // Reports the failure, then never exits. + child.stderr.emit('data', WINDOWS_LOCK_FAILURE) + return + } + child.emit('exit', 0, null) + }) + return child + } + const moveAside = vi.fn(async () => undefined) + + const result = await runWithLockRecovery('/node', ['/pnpm.cjs', 'add', 'x'], { + spawnProcess, + moveAside, + exists: () => true, + listEntries: async () => [], + wait: async () => undefined, + now: () => 99, + idleTimeoutMs: 10_000, + stallAfterFailureMs: 5, + killGraceMs: 5, + kill: (child) => child.kill(), + watchActivity: () => () => undefined, + report: () => undefined + }) + + expect(result.code).toBe(0) + expect(calls).toHaveLength(3) + expect(moveAside).toHaveBeenCalledWith( + BLOCKED_TARGET, + `${BLOCKED_TARGET}${SIDELINE_MARKER}99` + ) + }) + + it('gives up on a run whose kill never lands', async () => { + // taskkill can miss on Windows. Waiting for an exit that never comes would + // be the very hang the idle timeout exists to prevent. + const { spawnProcess } = fakePnpm([{ silent: true }]) + + const result = await runWithLockRecovery('/node', ['/pnpm.cjs', 'add', 'x'], { + spawnProcess, + idleTimeoutMs: 5, + killGraceMs: 5, + kill: () => undefined, + watchActivity: () => () => undefined, + report: () => undefined + }) + + expect(result.code).toBe(1) + expect(result.idleTimedOut).toBe(true) + }) + + it('says what it did, so a report without those lines names an unwrapped pnpm', async () => { + const { spawnProcess } = fakePnpm([ + { code: 1, output: WINDOWS_LOCK_FAILURE }, + { code: 1, output: WINDOWS_LOCK_FAILURE }, + { code: 0, output: '' } + ]) + const lines = [] + + await runWithLockRecovery('/node', ['/pnpm.cjs', 'add', 'x'], { + spawnProcess, + moveAside: async () => undefined, + exists: () => true, + listEntries: async () => [], + wait: async () => undefined, + now: () => 1234, + report: (message) => lines.push(message) + }) + + expect(lines.join('\n')).toContain('retrying') + const freed = lines.find((line) => line.startsWith('freed ')) + expect(freed).toContain(BLOCKED_TARGET) + expect(lines.at(-1)).toContain('succeeded') + expect(MARKER).toContain('dsh-desktop') + }) + + it('reports pnpm\u2019s own failure when the directory cannot be moved either', async () => { + const { spawnProcess, calls } = fakePnpm([ + { code: 1, output: WINDOWS_LOCK_FAILURE }, + { code: 1, output: WINDOWS_LOCK_FAILURE } + ]) + + const result = await runWithLockRecovery('/node', ['/pnpm.cjs', 'add', 'x'], { + spawnProcess, + moveAside: async () => { + throw new Error('EPERM') + }, + exists: () => true, + listEntries: async () => [], + wait: async () => undefined + }) + + expect(result.code).toBe(1) + expect(result.output).toContain('EPERM') + expect(calls).toHaveLength(2) + }) + + it('frees every destination the run staged for, not only the one pnpm named', async () => { + // pnpm reports the first blocked destination its workers hit, but an + // update blocks on every package it has to replace. Freeing one per + // attempt cannot finish an update that replaces four of them. + const tree = { + [modules()]: [ + directory('dshmarket'), + directory('dshmarket_tmp_7408_13'), + directory('layout-base'), + directory('layout-base_tmp_7408_4'), + directory('cytoscape-fcose'), + directory('.pnpm'), + directory('@scope') + ], + [modules('@scope')]: [directory('inner'), directory('inner_tmp_7408_9')], + [modules('cytoscape-fcose', 'node_modules')]: [directory('cose-base_tmp_7408_7')] + } + + const targets = await blockedTargets(modules('argparse'), ROOT, { + listEntries: async (path) => tree[path] ?? [], + exists: () => true + }) + + // pnpm's own diagnosis leads; the rest come from the staging left behind, + // nested node_modules and scoped packages included. + expect(targets).toEqual([ + modules('argparse'), + modules('dshmarket'), + modules('layout-base'), + modules('cytoscape-fcose', 'node_modules', 'cose-base'), + modules('@scope', 'inner') + ]) + }) + + it('offers only destinations that are actually there', async () => { + const targets = await blockedTargets(undefined, ROOT, { + listEntries: async (path) => + path === modules() ? [directory('gone_tmp_1_1'), directory('kept_tmp_1_1')] : [], + exists: (path) => path.endsWith('kept') + }) + + expect(targets).toEqual([modules('kept')]) + }) + + it('does not mistake a sidelined copy for staging', async () => { + // `.dsh-old-` is this runner's own leftover. Deriving a target + // from it would name a package that was never being replaced. + const targets = await blockedTargets(undefined, ROOT, { + listEntries: async (path) => + path === modules() ? [directory(`cose-base${SIDELINE_MARKER}17`)] : [], + exists: () => true + }) + + expect(targets).toEqual([]) + }) +}) + +const ROOT = join('/', 'p') + +/** A path under the fake profile's node_modules, in the host's own separators. */ +function modules(...segments) { + return join(ROOT, 'node_modules', ...segments) +} + +function directory(name) { + return { name, isDirectory: () => true, isSymbolicLink: () => false } +} diff --git a/test/profile-plugin-command.test.ts b/test/profile-plugin-command.test.ts index 9a5f3138..745a2284 100644 --- a/test/profile-plugin-command.test.ts +++ b/test/profile-plugin-command.test.ts @@ -1,7 +1,19 @@ import { mkdir, readFile, rm, writeFile } from 'node:fs/promises' import { join } from 'node:path' import { afterEach, describe, expect, it } from 'vitest' -import { removeProfilePluginWithDsh } from '../src/main/runtime/profile-plugin-command' +import { + buildPnpmShimCommand, + diagnosticLine, + removeProfilePluginWithDsh +} from '../src/main/runtime/profile-plugin-command' + +const existingRunnerPath = join( + __dirname, + '..', + 'packages', + 'dsh-desktop-market-installer', + 'pnpm-runner.mjs' +) describe('profile-plugin-command', () => { const testDir = join(__dirname, '.temp-profile-plugin-command-test') @@ -56,3 +68,41 @@ describe('profile-plugin-command', () => { }) }) }) + +describe('profile pnpm shim and failure reporting', () => { + it('keeps the desktop shim on the same lock-recovery runner Harness uses', () => { + // Both writers share /.desktop-bin, so a desktop-written shim + // that called pnpm directly would silently drop the recovery until + // Harness next rewrote them. + const base = { + dshHome: '/home/.dsh', + dshEntryPath: '/app/dsh/bin.js', + nodeExecutablePath: '/app/node', + pnpmEntryPath: '/app/pnpm.cjs' + } + + expect(buildPnpmShimCommand(base)).toEqual(['/app/pnpm.cjs']) + expect( + buildPnpmShimCommand({ ...base, pnpmRunnerPath: '/app/missing-runner.mjs' }) + ).toEqual(['/app/pnpm.cjs']) + expect( + buildPnpmShimCommand({ ...base, pnpmRunnerPath: existingRunnerPath }) + ).toEqual([existingRunnerPath, '/app/pnpm.cjs']) + }) + + it('reports the failure that names a cause, not dsh’s wrapper line', () => { + // dsh always ends with "pnpm failed in profile directory …", which names + // nothing — reporting that turns every failure into a dead end. + expect( + diagnosticLine( + [ + 'Progress: resolved 120, reused 118', + "error: EPERM: operation not permitted, rename 'x_tmp_1_1' -> 'x'", + 'dsh: pnpm failed in profile directory C:\\profiles\\web' + ].join('\n') + ) + ).toContain('EPERM') + expect(diagnosticLine('a\nb\nlast line')).toBe('last line') + expect(diagnosticLine(' ')).toBeUndefined() + }) +}) diff --git a/test/profile-repair.test.ts b/test/profile-repair.test.ts new file mode 100644 index 00000000..2d974aa2 --- /dev/null +++ b/test/profile-repair.test.ts @@ -0,0 +1,162 @@ +import { mkdir, mkdtemp, rm, symlink, writeFile } from 'node:fs/promises' +import { existsSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, describe, expect, it } from 'vitest' +import { + clearDamagedPackageDirectories, + findDamagedPackageDirectories, + hasProfile +} from '../src/main/state/profile-repair' +import { buildProfileInstallArguments } from '../src/main/runtime/profile-plugin-command' + +describe('profile repair', () => { + const homes: string[] = [] + + async function profileHome(): Promise<{ home: string; nodeModules: string }> { + const home = await mkdtemp(join(tmpdir(), 'dsh-profile-repair-')) + homes.push(home) + const profile = join(home, 'profiles', 'web') + const nodeModules = join(profile, 'node_modules') + await mkdir(nodeModules, { recursive: true }) + await writeFile( + join(profile, 'package.json'), + JSON.stringify({ dependencies: { dshmarket: '1.15.0' } }), + 'utf8' + ) + return { home, nodeModules } + } + + async function materialize(directory: string, name: string): Promise { + const path = join(directory, name) + await mkdir(path, { recursive: true }) + await writeFile(join(path, 'package.json'), JSON.stringify({ name, version: '1.0.0' }), 'utf8') + return path + } + + afterEach(async () => { + await Promise.all(homes.splice(0).map((home) => rm(home, { recursive: true, force: true }))) + }) + + it('reports nothing for a profile whose packages are all materialized', async () => { + const { home, nodeModules } = await profileHome() + await materialize(nodeModules, 'dshmarket') + await materialize(join(nodeModules, '@deepseek-ai'), 'dsh-settings') + + await expect(findDamagedPackageDirectories(home)).resolves.toEqual([]) + }) + + it('finds the leftovers a failed pnpm run blocks the next one with', async () => { + const { home, nodeModules } = await profileHome() + // What the Windows locked rename leaves: a name taken by something that is + // not a package, which pnpm can then never rename its staging onto. + const halfWritten = join(nodeModules, 'cose-base') + await mkdir(halfWritten, { recursive: true }) + await writeFile(join(halfWritten, 'index.js'), '', 'utf8') + const staging = join(nodeModules, 'cose-base_tmp_15968_6') + await mkdir(staging, { recursive: true }) + await writeFile(join(staging, 'package.json'), JSON.stringify({ name: 'cose-base' }), 'utf8') + const sidelined = join(nodeModules, 'argparse.dsh-old-1787317710932') + await materialize(nodeModules, 'argparse') + await mkdir(sidelined, { recursive: true }) + + const damaged = await findDamagedPackageDirectories(home) + + expect(new Set(damaged)).toEqual(new Set([halfWritten, staging, sidelined])) + }) + + it('spares pnpm’s own entries, scoped packages, and symlinks', async () => { + const { home, nodeModules } = await profileHome() + await mkdir(join(nodeModules, '.pnpm'), { recursive: true }) + await mkdir(join(nodeModules, '.bin'), { recursive: true }) + // A scope directory holds packages and has no manifest of its own. + await materialize(join(nodeModules, '@deepseek-ai'), 'dsh-settings') + const target = await materialize(nodeModules, 'dshmarket') + // Directory symlinks need Developer Mode or elevation on Windows; the rest + // of the expectation still holds where they cannot be created. + await symlink(target, join(nodeModules, 'linked-plugin'), 'junction').catch(() => undefined) + + await expect(findDamagedPackageDirectories(home)).resolves.toEqual([]) + }) + + it('finds a damaged package inside a scope', async () => { + const { home, nodeModules } = await profileHome() + const damagedScoped = join(nodeModules, '@linxin666', 'dsh-web-ui-all') + await mkdir(damagedScoped, { recursive: true }) + + await expect(findDamagedPackageDirectories(home)).resolves.toEqual([damagedScoped]) + }) + + it('clears what it found so the install can claim the names again', async () => { + const { home, nodeModules } = await profileHome() + const halfWritten = join(nodeModules, 'cose-base') + await mkdir(halfWritten, { recursive: true }) + const intact = await materialize(nodeModules, 'dshmarket') + + await expect(clearDamagedPackageDirectories(home)).resolves.toEqual([halfWritten]) + expect(existsSync(halfWritten)).toBe(false) + expect(existsSync(intact)).toBe(true) + await expect(findDamagedPackageDirectories(home)).resolves.toEqual([]) + }) + + it('finds the leftovers inside a package’s own node_modules', async () => { + // Once the package being replaced is a dependency of a dependency, its + // staging lands under the dependent, not at the top level — and a sweep + // that stops at the top level leaves one copy behind per attempt. + const { home, nodeModules } = await profileHome() + const dependent = await materialize(nodeModules, 'cytoscape-fcose') + const nested = join(dependent, 'node_modules') + await materialize(nested, 'cose-base') + const sidelined = join(nested, 'cose-base.dsh-old-1787327060846') + await mkdir(sidelined, { recursive: true }) + const staging = join(nested, 'cose-base_tmp_7408_7') + await mkdir(staging, { recursive: true }) + + const damaged = await findDamagedPackageDirectories(home) + + expect(new Set(damaged)).toEqual(new Set([sidelined, staging])) + await expect(clearDamagedPackageDirectories(home)).resolves.toHaveLength(2) + expect(existsSync(sidelined)).toBe(false) + expect(existsSync(staging)).toBe(false) + expect(existsSync(join(nested, 'cose-base'))).toBe(true) + }) + + it('reports only the directories that are actually gone', async () => { + // Node's recursive `rm` resolves successfully without removing anything + // under a non-ASCII path, and a sweep that trusted it announced twelve + // cleared directories over a profile where all twelve were still there. + // Whatever the platform does, the count has to match the disk. + const { home, nodeModules } = await profileHome() + const staging = join(nodeModules, 'dshmarket_tmp_7408_13') + await mkdir(join(staging, 'lib'), { recursive: true }) + await writeFile(join(staging, 'lib', 'index.js'), '', 'utf8') + + const cleared = await clearDamagedPackageDirectories(home) + + expect(cleared).toEqual([staging]) + expect(existsSync(staging)).toBe(false) + }) + + it('leaves a home without a profile alone', async () => { + const home = await mkdtemp(join(tmpdir(), 'dsh-profile-repair-empty-')) + homes.push(home) + + expect(hasProfile(home)).toBe(false) + await expect(clearDamagedPackageDirectories(home)).resolves.toEqual([]) + }) + + it('restores the cleared packages through the profile’s own installer', () => { + // The repair runs with CI set, which is pnpm's cue to freeze the lockfile. + // A profile worth repairing is one where a failed `add` already wrote the + // new version into the lockfile while package.json still names the old, so + // freezing there fails on the divergence instead of healing it. + expect(buildProfileInstallArguments('/app/dsh/bin.js')).toEqual([ + '/app/dsh/bin.js', + 'plugin', + '--profile', + 'web', + 'install', + '--no-frozen-lockfile' + ]) + }) +}) diff --git a/test/release.test.ts b/test/release.test.ts index 3c0a8bc1..20665535 100644 --- a/test/release.test.ts +++ b/test/release.test.ts @@ -278,10 +278,10 @@ describe('GitHub release contract', () => { expect(workflow.match(/CSC_IDENTITY_AUTO_DISCOVERY: 'false'/g)).toHaveLength(2) expect(workflow).not.toContain("CSC_LINK: ''") expect(workflow).toMatch( - /macos-apple-silicon:\r?\n name: macOS Apple Silicon\r?\n runs-on: macos-15\r?\n steps:/ + /macos-apple-silicon:\r?\n\s+name: macOS Apple Silicon\r?\n(?:[\s\S]*?)runs-on: macos-15\r?\n\s+steps:/ ) expect(workflow).toMatch( - /macos-intel:\r?\n name: macOS Intel\r?\n if: [^\r\n]+\r?\n runs-on: macos-15-intel\r?\n steps:/ + /macos-intel:\r?\n\s+name: macOS Intel\r?\n(?:[\s\S]*?)runs-on: macos-15-intel\r?\n\s+steps:/ ) }) diff --git a/test/remove-tree.test.ts b/test/remove-tree.test.ts new file mode 100644 index 00000000..da1b4dae --- /dev/null +++ b/test/remove-tree.test.ts @@ -0,0 +1,99 @@ +import { existsSync } from 'node:fs' +import { mkdir, mkdtemp, readdir, rm, symlink, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterAll, describe, expect, it } from 'vitest' + +import { removeTree, removeTreeIfPossible } from '../src/main/state/remove-tree' + +const roots: string[] = [] + +async function scratch(name: string): Promise { + const root = await mkdtemp(join(tmpdir(), `dsh-remove-tree-${name}-`)) + roots.push(root) + return root +} + +/** A profile-shaped tree: nested directories, files, and an empty directory. */ +async function packageTree(root: string, name: string): Promise { + const pkg = join(root, name) + await mkdir(join(pkg, 'lib', 'types'), { recursive: true }) + await mkdir(join(pkg, 'empty'), { recursive: true }) + await writeFile(join(pkg, 'package.json'), '{"name":"x"}') + await writeFile(join(pkg, 'lib', 'index.js'), 'export default 1') + await writeFile(join(pkg, 'lib', 'types', 'index.d.ts'), 'export {}') + return pkg +} + +afterAll(async () => { + for (const root of roots) await rm(root, { recursive: true, force: true }).catch(() => undefined) +}) + +describe('removeTree', () => { + it('removes a package tree whose path is not ASCII', async () => { + // The reason this module exists. Node's recursive `rm` reports success and + // removes nothing under such a path on Windows, so every cleanup built on + // it silently stopped working for anyone whose account name is not ASCII. + const root = await scratch('unicode') + const home = join(root, '数据项素', 'node_modules') + await mkdir(home, { recursive: true }) + const pkg = await packageTree(home, 'cose-base') + + await removeTree(pkg) + + expect(existsSync(pkg)).toBe(false) + expect(await readdir(home)).toEqual([]) + }) + + it('removes an ASCII tree the same way', async () => { + const root = await scratch('ascii') + const pkg = await packageTree(root, 'layout-base') + + await removeTree(pkg) + + expect(existsSync(pkg)).toBe(false) + }) + + it('treats a missing path as already removed', async () => { + const root = await scratch('missing') + + await expect(removeTree(join(root, 'never-there'))).resolves.toBeUndefined() + }) + + it('detaches a symlink without following it', async () => { + // pnpm's layout links into a shared store. Following one would take the + // store's contents with it, which is a far worse outcome than a leftover. + const root = await scratch('symlink') + const store = await packageTree(root, 'store-copy') + const link = join(root, 'linked') + try { + await symlink(store, link, 'junction') + } catch { + return // an unprivileged Windows session cannot create links; nothing to assert + } + + await removeTree(link) + + expect(existsSync(link)).toBe(false) + expect(existsSync(join(store, 'package.json'))).toBe(true) + }) + + it('removes a single file', async () => { + const root = await scratch('file') + const file = join(root, 'pnpm-lock.yaml') + await writeFile(file, 'lockfileVersion: 9') + + await removeTree(file) + + expect(existsSync(file)).toBe(false) + }) + + it('reports whether the path is gone rather than throwing', async () => { + const root = await scratch('report') + const pkg = await packageTree(root, 'dshmarket') + + await expect(removeTreeIfPossible(pkg)).resolves.toBe(true) + await expect(removeTreeIfPossible(join(root, 'absent'))).resolves.toBe(true) + }) + +}) diff --git a/test/runtime.test.ts b/test/runtime.test.ts index 7db8e1ff..b5b944fd 100644 --- a/test/runtime.test.ts +++ b/test/runtime.test.ts @@ -1,3 +1,5 @@ +import { readFile } from 'node:fs/promises' +import { join } from 'node:path' import { describe, expect, it } from 'vitest' import { buildHarnessArguments, @@ -159,6 +161,25 @@ describe('Harness launch contract', () => { ).toBe(false) }) + it('declares Node mode for Harness children without imposing it on the utility process', async () => { + // dsh-market re-runs the dsh CLI as `execPath [...execArgv] bin.js plugin + // --profile web add …`. On macOS execPath is the Electron helper, so + // without Node mode that child boots as an Electron app, the leading + // `--expose-internals` shifts argv, and the CLI answers "--profile + // is required" instead of installing. The flag cannot travel in the + // process environment: the utility process is launched with Chromium + // switches Node rejects, so the entry sets it from the inside instead. + const macOptions = buildHarnessSpawnOptions('/launch-root', '/harness', 'darwin', { + PATH: '/usr/bin', + ELECTRON_RUN_AS_NODE: '1' + }) + expect(macOptions.env).not.toHaveProperty('ELECTRON_RUN_AS_NODE') + + const entry = await readFile(join(process.cwd(), 'build', 'harness-node-entry.mjs'), 'utf8') + expect(entry).toContain('process.versions.electron !== undefined') + expect(entry).toContain("process.env.ELECTRON_RUN_AS_NODE = '1'") + }) + it('rejects an unexpected macOS Harness argument layout', () => { expect(() => buildDisclaimedUtilityProcessSpec(['entry.mjs'], {