Skip to content

Commit 63a20c1

Browse files
authored
fix(test): treat exited zombie panes as stopped during cleanup (#107)
An exited Linux pane can keep its `/proc` entry and start tick while waiting to be reaped. The tmux test harness treated that zombie as a running process after pidfd had already signaled exit, so rollback could report a surviving pane or attempt escalation against an exited process. Reuse the existing proc-state parser to exclude zombies while retaining exact generation checks. Add a real fork/waitid regression that holds a child unreaped and covers running, stale-generation, zombie, and reaped states. The regression fails before the fix and passes after it; production tmux control is unchanged. Validation: `npm run verify` passed on Node 24.18.0: 1,448 server tests, 49 real tmux/PTY tests, 521 client tests, and 29 Rust tests, with zero failures or skips. Type checks, lint, identity, audit at the existing threshold, and the production build passed. Local release metadata validation also passed. Related investigation: #105 failed its rollback assertion with an aggregate cleanup error in run 33912030795. That intermittent CI failure did not reproduce in 350 local startup/rollback iterations; this PR proves and fixes the zombie boundary independently. Release-grade CUA was not run.
1 parent 2a44ae3 commit 63a20c1

2 files changed

Lines changed: 53 additions & 5 deletions

File tree

server/modules/providers/tests/support/tmux-owned-server.ts

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ import { promisify } from 'node:util';
66

77
import { spawn as spawnPty, type IPty } from 'node-pty';
88

9+
import { parseProcStatState } from '../../services/process-start-time.service.js';
10+
911
const execFileAsync = promisify(execFile);
1012
const EXIT_TIMEOUT_MS = 5_000;
1113

@@ -33,10 +35,12 @@ async function readOwnedProcess(pid: number): Promise<OwnedPaneProcess> {
3335
return { pid, processGroupId, startedAtTicks: processStartTicks(stat) };
3436
}
3537

36-
async function stillOwned(process: OwnedPaneProcess): Promise<boolean> {
38+
export async function isOwnedPaneRunning(process: OwnedPaneProcess): Promise<boolean> {
3739
try {
3840
const stat = await import('node:fs/promises').then(({ readFile }) => readFile(`/proc/${process.pid}/stat`, 'utf8'));
39-
return processStartTicks(stat) === process.startedAtTicks;
41+
// pidfd signals exit before the parent necessarily reaps /proc. A zombie
42+
// retains its start tick but cannot run or receive an escalation signal.
43+
return processStartTicks(stat) === process.startedAtTicks && parseProcStatState(stat) !== 'Z';
4044
} catch (error) {
4145
if (error instanceof Error && 'code' in error && (error.code === 'ENOENT' || error.code === 'ESRCH')) return false;
4246
throw error;
@@ -154,7 +158,7 @@ export async function startOwnedTmuxServer(
154158
try { await run(['kill-server']); } catch { pty.kill('SIGTERM'); }
155159
const escalation = setTimeout(() => {
156160
for (const owned of panes) {
157-
void stillOwned(owned).then((ownedNow) => {
161+
void isOwnedPaneRunning(owned).then((ownedNow) => {
158162
if (!ownedNow) return;
159163
try { process.kill(-owned.processGroupId, 'SIGKILL'); } catch (error) {
160164
if (!(error instanceof Error && 'code' in error && error.code === 'ESRCH')) throw error;
@@ -166,7 +170,7 @@ export async function startOwnedTmuxServer(
166170
await Promise.all([ptyExit.exited, ...paneExits.map(({ exited }) => exited)]);
167171
clearTimeout(escalation);
168172
for (const owned of panes) {
169-
if (await stillOwned(owned)) throw new OwnedTmuxError(`Owned pane PID ${owned.pid} survived tmux shutdown.`);
173+
if (await isOwnedPaneRunning(owned)) throw new OwnedTmuxError(`Owned pane PID ${owned.pid} survived tmux shutdown.`);
170174
}
171175
try { process.kill(pty.pid, 0); } catch (error) {
172176
if (error instanceof Error && 'code' in error && error.code === 'ESRCH') return;

server/modules/providers/tests/tmux-fleet-lifecycle.e2e.test.ts

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,16 @@
11
import assert from 'node:assert/strict';
22
import { spawn, spawnSync, type ChildProcessWithoutNullStreams } from 'node:child_process';
3-
import { stat } from 'node:fs/promises';
3+
import { readFile, stat } from 'node:fs/promises';
44
import path from 'node:path';
55
import test from 'node:test';
66
import { pathToFileURL } from 'node:url';
77

88
import { startFleetServers, stopFleetProcesses, type FleetProcess } from '../../../../scripts/cua/fleet-process-lifecycle.js';
9+
import { parseProcStatStartTicks, parseProcStatState } from '../services/process-start-time.service.js';
910

1011
import { createTmuxFleetE2EHarness } from './support/tmux-fleet-harness.js';
1112
import { createTmuxFleetNode } from './support/tmux-fleet-node.js';
13+
import { isOwnedPaneRunning } from './support/tmux-owned-server.js';
1214
import type { TmuxFleetNode } from './support/tmux-e2e-types.js';
1315

1416
const tmuxE2ESkip = process.platform === 'win32'
@@ -70,6 +72,48 @@ function startFleetWorker(): ChildProcessWithoutNullStreams {
7072
});
7173
}
7274

75+
test('owned pane cleanup distinguishes an exited zombie from a running generation', {
76+
skip: process.platform !== 'linux', timeout: 10_000, concurrency: false,
77+
}, async (t) => {
78+
// Keep an exited child unreaped until the assertion, just as pidfd readiness
79+
// can precede its parent's waitpid during concurrent tmux shutdown.
80+
const source = [
81+
'import os,sys',
82+
'reader,writer=os.pipe()',
83+
'pid=os.fork()',
84+
'if pid==0:',
85+
' os.close(writer); os.read(reader,1); os._exit(0)',
86+
'os.close(reader)',
87+
"print('LIVE='+str(pid),flush=True)",
88+
'try:',
89+
' sys.stdin.readline(); os.close(writer)',
90+
' os.waitid(os.P_PID,pid,os.WEXITED|os.WNOWAIT)',
91+
" print('ZOMBIE=',flush=True)",
92+
' sys.stdin.readline()',
93+
'finally: os.waitpid(pid,0)',
94+
].join('\n');
95+
const child = spawn('python3', ['-c', source], { stdio: ['pipe', 'pipe', 'pipe'] });
96+
const exited = waitForExit(child);
97+
t.after(async () => { child.stdin.end(); await exited; });
98+
const pid = Number(await waitForLine(child, 'LIVE='));
99+
const initialStat = await readFile(`/proc/${pid}/stat`, 'utf8');
100+
const ticks = parseProcStatStartTicks(initialStat);
101+
assert.ok(ticks !== null);
102+
const identity = { pid, processGroupId: pid, startedAtTicks: String(ticks) };
103+
assert.equal(await isOwnedPaneRunning(identity), true);
104+
assert.equal(await isOwnedPaneRunning({ ...identity, startedAtTicks: String(ticks + 1) }), false);
105+
106+
const zombie = waitForLine(child, 'ZOMBIE=');
107+
child.stdin.write('EXIT\n');
108+
await zombie;
109+
assert.equal(parseProcStatState(await readFile(`/proc/${pid}/stat`, 'utf8')), 'Z');
110+
assert.equal(await isOwnedPaneRunning(identity), false, 'an exited PID must not be escalated or reported as surviving shutdown');
111+
112+
child.stdin.end();
113+
await exited;
114+
assert.equal(await isOwnedPaneRunning(identity), false);
115+
});
116+
73117
test('fleet harness rolls back fulfilled siblings when one concurrent node start fails', {
74118
skip: tmuxE2ESkip, timeout: 30_000, concurrency: false,
75119
}, async () => {

0 commit comments

Comments
 (0)