fix(ci): repair desktop RPM dependency and teardown checks (#16154)

* fix(ci): repair desktop rpm dependency and teardown checks

Co-authored-by: Qwen-Coder <[email protected]>

* test(desktop): use a counted space class in the EPEL checksum pin

`scripts/lint.js --eslint` runs on every file the PR touches, and the new
rpm dependency test spelled the two spaces in the pinned `echo` line as
literals: `no-regex-spaces` rejects them, so Lint & Static failed on the
incremental ESLint step.

`[0-9a-f]{64} {2}` matches the same two spaces the workflow emits; the
assertion still pins exactly what the release step writes.

Co-authored-by: Qwen-Coder <[email protected]>
Patrol-Run: qwen-pr-closeout/jmvzj4h0j9a

* fix(desktop): keep the runtime group as a teardown target

Teardown collected daemon pids through the zombie-aware liveness predicate and
escalated SIGKILL only on leaders that were still alive, so a leader that had
already exited — reaped, replaced, or an unreaped zombie — silently dropped its
process group, which is teardown's only handle on it. The children the leader
spawned (MCP servers, tool subprocesses) then survived and kept writing under
the workspace about to be deleted, and the smoke reported a pass.

Collect every recorded pid, escalate on every collected group after the drain,
and keep the zombie-aware predicate on the "survived teardown" report so the
false positive this PR exists to fix stays fixed. The new fixture drives a
runtime whose leader exits while a writer in its group ignores SIGTERM.

Also pins the EPEL download URL and digest literals, drops the duplicate local
liveness predicate in favour of the shared helper, and covers the signal-probe
error arm (ESRCH gone; EPERM/EACCES alive, the rule the core helper documents).

Co-authored-by: Qwen-Coder <[email protected]>
Patrol-Run: qwen-pr-closeout/jmvzpjyde9j

* fix(desktop): decide teardown on the runtime group, not its leader

The round-2 escalation signals the recorded pid's group (`kill(-pid)`), but
the verdict and the drain still asked `isProcessAlive(pid)`, which answers
"gone" as soon as the leader exits — the exact topology
`SMOKE_FIXTURE_EXIT_AFTER_SPAWN` reproduces. In that state `waitForExit`
returned on its first poll, so the 5 s drain was skipped and the
`survived teardown` branch was unreachable.

`isTeardownTargetAlive` probes the group first and falls back to the leader
where there is no group to signal (win32), so the drain and the leak verdict
cover the children the group can still hold.

Also drops the "mirrors the canonical … keep both in sync" claim from the
desktop probe's comment: it diverges from core's `isPidAlive` on the
pid <= 0 guard, the zombie exclusion on the EPERM/EACCES arm, and the
vanished-/proc answer. The comment now states those differences and why
teardown wants the dead-on-ENOENT direction.

Co-authored-by: Qwen-Coder <[email protected]>
Patrol-Run: qwen-pr-closeout/jmvzrp44z9n

* fix(desktop): stop counting unreaped zombies as a live teardown group

`waitForExit` asks `isTeardownTargetAlive` whether a daemon's group still
exists. `kill(-pid, 0)` also answers for members that can no longer write, so
a daemon that exited and that the container's PID 1 has not reaped kept its
group "alive": the drain burned its five-second window, the second probe still
returned the pid, and teardown reported "the packaged runtime survived
teardown" for a runtime that was already gone.

On Linux the group probe now counts only members that can still write: scan
`/proc` for entries whose pgrp is the recorded pid, and report the group
stopped when every member is a zombie. A scan that observes no member keeps
the group probe's answer, so a hidden /proc cannot end a drain early, and the
group handle still wins over the leader handle.

Measured on a real zombie group (setsid + _exit, parent never reaps): /proc
state Z, `kill(-pgid, 0)` exit 0, `isProcessAlive` false, and
`isTeardownTargetAlive` true before this change, false after it.

Co-authored-by: Qwen-Coder <[email protected]>
Patrol-Run: qwen-pr-closeout/jmvzy4lf49w

---------

Co-authored-by: Qwen-Coder <[email protected]>
This commit is contained in:
易良andQwen-Coder authored and GitHub committed 2026-11-03 10:18:16 +08:00
1 parent 90eb13e4c6
commit 5b7f9ec1c3
5 files changed
+352 -52

No files matched your search

+3 -3
View File
@@ -474,9 +474,9 @@ jobs:
if [ '${{ matrix.epel }}' = 'true' ]; then
dnf install -y dnf-plugins-core
dnf config-manager --set-enabled crb
epel_rpm="$RUNNER_TEMP/epel-release-latest-10.noarch.rpm"
curl -fsSL --connect-timeout 15 --max-time 120 -o "$epel_rpm" 'https://dl.fedoraproject.org/pub/epel/epel-release-latest-10.noarch.rpm'
echo '8090214eb8b95617118478304aa5a8f796d4c19c625bc09e0c740cce0e682a34 '"$epel_rpm" | sha256sum -c -
epel_rpm="$RUNNER_TEMP/epel-release-10-9.el10_3.noarch.rpm"
curl -fsSL --connect-timeout 15 --max-time 120 -o "$epel_rpm" 'https://dl.fedoraproject.org/pub/epel/10/Everything/x86_64/Packages/e/epel-release-10-9.el10_3.noarch.rpm'
echo 'f567aa5c3cde55f579ba5c41c79df97da692a391efddd14b123a4eec0824fbc0 '"$epel_rpm" | sha256sum -c -
dnf install -y "$epel_rpm"
fi
dnf install -y --setopt=install_weak_deps=False \
@@ -0,0 +1,93 @@
import fs from 'node:fs';
// Teardown-only probe, deliberately narrower than the canonical
// packages/core/src/utils/process-liveness.ts: it takes no `pid <= 0` guard,
// skips the zombie exclusion on the EPERM/EACCES arm, and reads a vanished
// /proc entry as dead — the safe answer when the question is whether this
// script's own runtime leaked.
export function isProcessAlive(
pid,
{
platform = process.platform,
kill = process.kill,
readStat = (statPath) => fs.readFileSync(statPath, 'utf8'),
} = {},
) {
try {
kill(pid, 0);
} catch (error) {
// EPERM/EACCES: it exists under another owner.
return error?.code === 'EPERM' || error?.code === 'EACCES';
}
if (platform !== 'linux') return true;
try {
const stat = readStat(`/proc/${pid}/stat`);
const closingParen = stat.lastIndexOf(')');
if (closingParen < 0 || stat[closingParen + 1] !== ' ') return true;
return stat[closingParen + 2] !== 'Z';
} catch (error) {
return error?.code !== 'ENOENT';
}
}
// A daemon record's pid is also its process group, and the group outlives the
// leader that recorded it: after the leader exits a bare pid probe answers
// "gone" while the group's children keep writing under the workspace.
export function isTeardownTargetAlive(
pid,
{
platform = process.platform,
kill = process.kill,
readDir = (dir) => fs.readdirSync(dir),
readStat = (statPath) => fs.readFileSync(statPath, 'utf8'),
} = {},
) {
if (platform === 'win32') return isProcessAlive(pid, { platform, kill });
try {
kill(-pid, 0);
} catch (error) {
if (error?.code === 'ESRCH') return false;
if (error?.code === 'EPERM' || error?.code === 'EACCES') return true;
return isProcessAlive(pid, { platform, kill });
}
// The group probe also answers for members that can no longer write: an
// exited daemon the container's PID 1 has not reaped keeps its group
// "alive" as a zombie, which would fail teardown on a runtime that is gone.
if (platform !== 'linux') return true;
return groupHasLiveMember(pid, { readDir, readStat });
}
function groupHasLiveMember(pgid, { readDir, readStat }) {
let entries;
try {
entries = readDir('/proc');
} catch {
// Membership unknown: keep the group probe's answer rather than ending a
// drain on a failed scan.
return true;
}
let members = 0;
for (const entry of entries) {
if (!/^\d+$/.test(entry)) continue;
let stat;
try {
stat = readStat(`/proc/${entry}/stat`);
} catch {
// The member exited between readdir and read, which is the wanted state.
continue;
}
const closingParen = stat.lastIndexOf(')');
if (closingParen < 0) continue;
// `/proc/<pid>/stat` after `comm`: state, ppid, pgrp, ...
const fields = stat
.slice(closingParen + 2)
.trim()
.split(' ');
if (fields.length < 3 || Number(fields[2]) !== pgid) continue;
if (fields[0] !== 'Z') return true;
members += 1;
}
// Nothing observed is not the same as "only zombies": when /proc hides the
// group, the group probe that answered alive stays the better evidence.
return members === 0;
}
+10 -16
View File
@@ -5,6 +5,7 @@ import fs from 'node:fs';
import os from 'node:os';
import path from 'node:path';
import { fileURLToPath } from 'node:url';
import { isTeardownTargetAlive } from './process-liveness.js';
import { resolveLogRoot, sliceNewLog } from './resolve-log-root.js';
const packageDir = path.resolve(
@@ -229,9 +230,11 @@ async function teardown(child, removeWorkspace) {
// recorded before the tree they live in is deleted.
const daemons = readDaemonPids();
for (const pid of daemons) terminate(pid);
let stragglers = await waitForExit(daemons, 5_000);
for (const pid of stragglers) terminate(pid, 'SIGKILL');
stragglers = await waitForExit(stragglers, 2_000);
await waitForExit(daemons, 5_000);
// A leader that already exited is not "alive", yet its group can still hold
// the children writing under the workspace: escalate on every group.
for (const pid of daemons) terminate(pid, 'SIGKILL');
const stragglers = await waitForExit(daemons, 2_000);
if (stragglers.length > 0) {
// A runtime we could not stop is a leak, not a filesystem race: fail the
// check instead of hiding it behind the delete retries below.
@@ -282,8 +285,9 @@ function readDaemonPids() {
continue;
}
for (const record of contents.matchAll(/\[DAEMON\][^\n]*\bpid=(\d+)/g)) {
const pid = Number(record[1]);
if (isAlive(pid)) pids.add(pid);
// The record is the only handle on the leader's group, which outlives
// the leader itself; `waitForExit` decides what actually stopped.
pids.add(Number(record[1]));
}
}
if (pids.size === 0 && fs.existsSync(daemonDir)) {
@@ -295,20 +299,10 @@ function readDaemonPids() {
return [...pids];
}
function isAlive(pid) {
try {
process.kill(pid, 0);
return true;
} catch (error) {
// EPERM means the process exists but belongs to another user.
return error.code === 'EPERM';
}
}
async function waitForExit(pids, timeoutMs) {
const deadline = Date.now() + timeoutMs;
for (;;) {
const alive = pids.filter(isAlive);
const alive = pids.filter(isTeardownTargetAlive);
if (alive.length === 0 || Date.now() >= deadline) return alive;
await new Promise((resolve) => setTimeout(resolve, 100));
}
+31
View File
@@ -76,6 +76,37 @@ const stepNamed = (name) =>
desktopJob.steps.find((step) => step.name === name) ??
desktopJob.steps.find((step) => String(step.uses ?? '').includes(name));
describe('desktop rpm build-root dependencies', () => {
it('checks a versioned EPEL release before installing it', () => {
const install = release.jobs.build.steps.find(
(step) => step.name === 'Install rpm build-root dependencies',
);
const source = install.run;
const download = source.match(/curl[^\n]+ '(https:\/\/[^']+)'/);
expect(download).not.toBeNull();
// Re-derive both literals from the real RPM (`sha256sum <rpm>`) whenever
// the versioned filename moves; nothing offline can prove they agree.
expect(download[1]).toBe(
'https://dl.fedoraproject.org/pub/epel/10/Everything/x86_64/Packages/e/epel-release-10-9.el10_3.noarch.rpm',
);
expect(source).toContain(
'f567aa5c3cde55f579ba5c41c79df97da692a391efddd14b123a4eec0824fbc0',
);
const filename = new URL(download[1]).pathname.split('/').pop();
expect(source).toContain(`epel_rpm="$RUNNER_TEMP/${filename}"`);
expect(source).toMatch(
/echo '[0-9a-f]{64} {2}'"\$epel_rpm" \| sha256sum -c -/,
);
expect(source).toContain('set -euo pipefail');
expect(source.indexOf('sha256sum -c -')).toBeGreaterThan(
source.indexOf('curl '),
);
expect(source.indexOf('dnf install -y "$epel_rpm"')).toBeGreaterThan(
source.indexOf('sha256sum -c -'),
);
});
});
describe('desktop isolation guard — nativePrefixes coverage', () => {
it('matches every negated native workspace entry', () => {
expect(npmNegations.length).toBeGreaterThan(0);
+215 -33
View File
@@ -10,6 +10,10 @@ import path from 'node:path';
import { fileURLToPath } from 'node:url';
import { afterAll, beforeAll, describe, expect, it } from 'vitest';
import { arePermissionBitsEnforced } from '../../packages/cli/src/test-utils/permission-bits.js';
import {
isProcessAlive,
isTeardownTargetAlive,
} from '../../packages/desktop/scripts/process-liveness.js';
const repoRoot = path.resolve(
path.dirname(fileURLToPath(import.meta.url)),
@@ -34,35 +38,56 @@ const runtimeScriptName = 'fake-runtime.js';
// workspace is deleted, the ENOTEMPTY/EBUSY race that failed the v0.24.0
// desktop build.
const FAKE_RUNTIME = `#!/usr/bin/env node
import { spawn } from 'node:child_process';
import fs from 'node:fs';
import path from 'node:path';
const debugDir = path.join(process.env.HOME, '.qwen', 'debug', 'daemon');
fs.mkdirSync(debugDir, { recursive: true });
fs.appendFileSync(
path.join(debugDir, 'daemon.log'),
\`2026-01-01T00:00:00.000Z [INFO] [DAEMON] runId=smoke-fixture pid=\${process.pid} starting\\n\`,
);
if (process.env.SMOKE_FIXTURE_PID_FILE) {
fs.writeFileSync(process.env.SMOKE_FIXTURE_PID_FILE, String(process.pid));
}
const write = () => {
try {
fs.appendFileSync(path.join(debugDir, 'trace.log'), Date.now() + '\\n');
} catch {}
};
process.on('SIGTERM', () => {
// A runtime that outlives the signal the app forwarded only ends on SIGKILL.
if (process.env.SMOKE_FIXTURE_STUBBORN === '1') return;
const deadline = Date.now() + 300;
const timer = setInterval(() => {
write();
if (Date.now() >= deadline) {
clearInterval(timer);
process.exit(0);
}
}, 25);
});
setInterval(write, 25);
if (process.env.SMOKE_FIXTURE_GRANDCHILD === '1') {
// A child of the runtime's group that only a group SIGKILL can end.
process.on('SIGTERM', () => {});
fs.writeFileSync(process.env.SMOKE_FIXTURE_READY_FILE, 'ready');
setInterval(write, 25);
} else {
fs.appendFileSync(
path.join(debugDir, 'daemon.log'),
\`2026-01-01T00:00:00.000Z [INFO] [DAEMON] runId=smoke-fixture pid=\${process.pid} starting\\n\`,
);
if (process.env.SMOKE_FIXTURE_PID_FILE) {
fs.writeFileSync(process.env.SMOKE_FIXTURE_PID_FILE, String(process.pid));
}
if (process.env.SMOKE_FIXTURE_EXIT_AFTER_SPAWN === '1') {
// The leader exits, so the recorded pid is the only handle on the group
// the writer it just spawned keeps running in.
const child = spawn(process.execPath, [process.argv[1]], {
stdio: 'ignore',
env: { ...process.env, SMOKE_FIXTURE_GRANDCHILD: '1' },
});
fs.writeFileSync(
process.env.SMOKE_FIXTURE_GRANDCHILD_PID_FILE,
String(child.pid),
);
process.exit(0);
}
process.on('SIGTERM', () => {
// A runtime that outlives the signal the app forwarded only ends on SIGKILL.
if (process.env.SMOKE_FIXTURE_STUBBORN === '1') return;
const deadline = Date.now() + 300;
const timer = setInterval(() => {
write();
if (Date.now() >= deadline) {
clearInterval(timer);
process.exit(0);
}
}, 25);
});
setInterval(write, 25);
}
`;
const FAKE_APP = `#!/usr/bin/env node
@@ -102,8 +127,12 @@ server.listen(0, '127.0.0.1', async () => {
// The smoke can only tear down once the daemon it has to stop is running, so
// wait for the runtime's own log before reporting readiness. A contended host
// can take seconds to exec the runtime; never report ready without it.
const readyMarker = process.env.SMOKE_FIXTURE_READY_FILE;
const deadline = Date.now() + 45_000;
while (!fs.existsSync(runtimeLog)) {
while (
!fs.existsSync(runtimeLog) ||
(readyMarker && !fs.existsSync(readyMarker))
) {
if (Date.now() >= deadline) {
console.error('fixture: the runtime never wrote ' + runtimeLog);
process.exit(4);
@@ -159,15 +188,6 @@ function recordedPid(pidFile) {
return Number(fs.readFileSync(pidFile, 'utf8'));
}
function isAlive(pid) {
try {
process.kill(pid, 0);
return true;
} catch (error) {
return error.code === 'EPERM';
}
}
// Keeps a failed assertion from leaving smoke workspaces behind, including the
// one a fixture locks against removal.
afterAll(() => {
@@ -198,6 +218,150 @@ beforeAll(() => {
);
});
describe('process liveness', () => {
it('treats Linux zombies as stopped while keeping live states alive', () => {
const readStat = (state) =>
`123 (runtime with spaces (and parentheses)) ${state} 1 2`;
const options = { platform: 'linux', kill: () => {} };
expect(
isProcessAlive(123, { ...options, readStat: () => readStat('Z') }),
).toBe(false);
expect(
isProcessAlive(123, { ...options, readStat: () => readStat('S') }),
).toBe(true);
});
it('keeps conservative liveness for missing or unreadable proc stats', () => {
const options = { platform: 'linux', kill: () => {} };
const missing = Object.assign(new Error('gone'), { code: 'ENOENT' });
const unreadable = Object.assign(new Error('denied'), { code: 'EACCES' });
expect(
isProcessAlive(123, {
...options,
readStat: () => {
throw missing;
},
}),
).toBe(false);
expect(
isProcessAlive(123, {
...options,
readStat: () => {
throw unreadable;
},
}),
).toBe(true);
for (const platform of ['darwin', 'win32']) {
let reads = 0;
expect(
isProcessAlive(123, {
platform,
kill: () => {},
readStat: () => {
reads += 1;
return '';
},
}),
).toBe(true);
expect(reads).toBe(0);
}
});
it('reads a failed signal probe by its error code', () => {
const withKillError = (code) => () => {
throw Object.assign(new Error(code), { code });
};
// ESRCH is the only answer that means the process is gone; EPERM/EACCES
// mean it exists under another owner, so teardown must not retire it.
expect(isProcessAlive(123, { kill: withKillError('ESRCH') })).toBe(false);
expect(isProcessAlive(123, { kill: withKillError('EPERM') })).toBe(true);
expect(isProcessAlive(123, { kill: withKillError('EACCES') })).toBe(true);
});
it('keeps a teardown target alive while its group outlives the leader', () => {
const leaderGoneGroupAlive = (pid) => {
if (pid < 0) return;
throw Object.assign(new Error('ESRCH'), { code: 'ESRCH' });
};
const groupGone = () => {
throw Object.assign(new Error('ESRCH'), { code: 'ESRCH' });
};
// The recorded pid is the group: reporting it dead on the leader alone
// ends the drain while the group's writer still runs.
expect(
isTeardownTargetAlive(123, {
platform: 'linux',
kill: leaderGoneGroupAlive,
}),
).toBe(true);
expect(
isTeardownTargetAlive(123, { platform: 'linux', kill: groupGone }),
).toBe(false);
});
// `kill(-pid, 0)` answers for members that can no longer write: a daemon the
// container's PID 1 has not reaped stays a zombie in its group, so the drain
// would wait out its window and fail teardown on a runtime that is gone.
it('reads a group of unreaped zombies as stopped', () => {
const groupAlive = () => {};
const procEntries = () => ['123', '124', 'self'];
const zombieStats = (statPath) => {
const pid = statPath.split('/')[2];
return `${pid} (qwen) Z 1 123 123 0 -1 0\n`;
};
const oneWriter = (statPath) => {
const pid = statPath.split('/')[2];
const state = pid === '124' ? 'S' : 'Z';
return `${pid} (qwen) ${state} 1 123 123 0 -1 0\n`;
};
expect(
isTeardownTargetAlive(123, {
platform: 'linux',
kill: groupAlive,
readDir: procEntries,
readStat: zombieStats,
}),
).toBe(false);
// The writer arm is what the group handle exists for: one live member
// keeps the group alive even though the leader itself is a zombie.
expect(
isTeardownTargetAlive(123, {
platform: 'linux',
kill: groupAlive,
readDir: procEntries,
readStat: oneWriter,
}),
).toBe(true);
});
it('keeps the platform arms the group probe falls back to', () => {
const leaderAliveGroupGone = (pid) => {
if (pid < 0) throw Object.assign(new Error('ESRCH'), { code: 'ESRCH' });
};
const groupDenied = (pid) => {
if (pid < 0) throw Object.assign(new Error('EPERM'), { code: 'EPERM' });
};
// Windows has no process groups and `terminate` uses taskkill, so the pid
// answer is the only one available there: a live leader stays alive even
// though the negative-pid probe is the one that fails.
expect(
isTeardownTargetAlive(123, {
platform: 'win32',
kill: leaderAliveGroupGone,
}),
).toBe(true);
expect(
isTeardownTargetAlive(123, { platform: 'linux', kill: groupDenied }),
).toBe(true);
});
});
// The smoke spawns the app path it is handed, and a shebang script is not
// spawnable on Windows, so this fixture topology only exists on POSIX. The
// Windows lane exercises the same teardown against the real packaged build.
@@ -210,7 +374,7 @@ describe.skipIf(process.platform === 'win32')('smoke-packaged teardown', () => {
expect(result.created, result.stderr).toEqual([]);
// The app's own signal never reaches the runtime's group; only the pid the
// daemon recorded can stop it.
expect(isAlive(recordedPid(pidFile))).toBe(false);
expect(isProcessAlive(recordedPid(pidFile))).toBe(false);
});
it('stops a runtime that ignores SIGTERM instead of racing its writes', () => {
@@ -221,7 +385,25 @@ describe.skipIf(process.platform === 'win32')('smoke-packaged teardown', () => {
});
expect(result.status, result.stderr).toBe(0);
expect(result.created, result.stderr).toEqual([]);
expect(isAlive(recordedPid(pidFile))).toBe(false);
expect(isProcessAlive(recordedPid(pidFile))).toBe(false);
});
// The recorded pid stays the teardown handle after its leader exits: a group
// whose leader is gone still runs the writer that outlives it.
it('reaps the group of a runtime whose leader already exited', () => {
const pidFile = path.join(fixtureRoot, 'exited-leader.pid');
const writerPidFile = path.join(fixtureRoot, 'exited-leader-writer.pid');
const result = runSmoke(fakeApp, {
SMOKE_FIXTURE_PID_FILE: pidFile,
SMOKE_FIXTURE_EXIT_AFTER_SPAWN: '1',
SMOKE_FIXTURE_GRANDCHILD_PID_FILE: writerPidFile,
SMOKE_FIXTURE_READY_FILE: path.join(fixtureRoot, 'exited-leader.ready'),
});
expect(result.status, result.stderr).toBe(0);
expect(result.created, result.stderr).toEqual([]);
expect(isProcessAlive(Number(fs.readFileSync(writerPidFile, 'utf8')))).toBe(
false,
);
});
it('still force-kills an app that holds SIGTERM before it cleans up', () => {
@@ -233,7 +415,7 @@ describe.skipIf(process.platform === 'win32')('smoke-packaged teardown', () => {
expect(result.status, result.stderr).toBe(0);
expect(result.stdout).toContain('Packaged desktop runtime ready');
expect(result.created, result.stderr).toEqual([]);
expect(isAlive(recordedPid(pidFile))).toBe(false);
expect(isProcessAlive(recordedPid(pidFile))).toBe(false);
});
// The locked directory only stops the removal where permission bits