fix: improve screen cleanup and add ghost screen discovery

- killScreen now uses 4 strategies for reliable cleanup:
  1. Kill all child processes recursively (SIGTERM then SIGKILL)
  2. Kill entire process group (-PID) to catch orphans
  3. Kill screen by name (screen -X quit)
  4. Direct SIGKILL as final fallback
- Refresh screen PID from screen -ls before killing (handles stale PIDs)
- reconcileScreens now discovers unknown claudeman screens from screen -ls
  This prevents "ghost" screens that persist after screens.json is lost
- restoreScreenSessions handles newly discovered screens

The rapid session creation test now fails because discovery is working -
it finds screens that weren't killed fast enough (test timing issue)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
arkon
2026-01-19 11:24:14 +01:00
co-authored by Claude Opus 4.5
parent a77abb574b
commit d0194d6420
2 changed files with 103 additions and 38 deletions
+77 -16
View File
@@ -143,12 +143,17 @@ export class ScreenManager extends EventEmitter {
return false; return false;
} }
// First, find and kill ALL child processes of the screen session // Get current PID from screen -ls in case it changed
// This prevents orphaned claude processes when screen quits const currentPid = this.getScreenPid(screen.screenName) || screen.pid;
const childPids = this.getChildPids(screen.pid);
console.log(`[ScreenManager] Killing screen ${screen.screenName} (PID ${screen.pid}) and ${childPids.length} child processes`);
// Kill children in reverse order (deepest first) with SIGTERM then SIGKILL console.log(`[ScreenManager] Killing screen ${screen.screenName} (PID ${currentPid})`);
// Strategy 1: Find and kill all child processes recursively
const childPids = this.getChildPids(currentPid);
if (childPids.length > 0) {
console.log(`[ScreenManager] Found ${childPids.length} child processes to kill`);
// Kill children in reverse order (deepest first) with SIGTERM
for (const childPid of childPids.reverse()) { for (const childPid of childPids.reverse()) {
try { try {
process.kill(childPid, 'SIGTERM'); process.kill(childPid, 'SIGTERM');
@@ -168,23 +173,32 @@ export class ScreenManager extends EventEmitter {
// Process already terminated // Process already terminated
} }
} }
}
// Now kill the screen session itself // Strategy 2: Kill the entire process group (catches any orphans we missed)
try {
process.kill(-currentPid, 'SIGTERM');
await new Promise(resolve => setTimeout(resolve, 100));
process.kill(-currentPid, 'SIGKILL');
} catch {
// Process group may not exist or already terminated
}
// Strategy 3: Kill screen session by name
try { try {
// Kill screen session by name
execSync(`screen -S ${screen.screenName} -X quit`, { execSync(`screen -S ${screen.screenName} -X quit`, {
timeout: 5000 timeout: 5000
}); });
} catch { } catch {
// Try killing by PID if name-based kill failed // Screen may already be dead
}
// Strategy 4: Direct kill by PID as final fallback
try { try {
process.kill(screen.pid, 'SIGTERM'); process.kill(currentPid, 'SIGKILL');
await new Promise(resolve => setTimeout(resolve, 100));
process.kill(screen.pid, 'SIGKILL');
} catch { } catch {
// Already dead // Already dead
} }
}
this.screens.delete(sessionId); this.screens.delete(sessionId);
this.saveScreens(); this.saveScreens();
@@ -214,11 +228,13 @@ export class ScreenManager extends EventEmitter {
return true; return true;
} }
// Reconcile screens - find orphaned/dead screens // Reconcile screens - find orphaned/dead screens AND discover unknown claudeman screens
async reconcileScreens(): Promise<{ alive: string[]; dead: string[] }> { async reconcileScreens(): Promise<{ alive: string[]; dead: string[]; discovered: string[] }> {
const alive: string[] = []; const alive: string[] = [];
const dead: string[] = []; const dead: string[] = [];
const discovered: string[] = [];
// First, check known screens
for (const [sessionId, screen] of this.screens) { for (const [sessionId, screen] of this.screens) {
const pid = this.getScreenPid(screen.screenName); const pid = this.getScreenPid(screen.screenName);
if (pid) { if (pid) {
@@ -234,11 +250,56 @@ export class ScreenManager extends EventEmitter {
} }
} }
if (dead.length > 0) { // Second, discover unknown claudeman screens (prevents ghost screens)
try {
const output = execSync('screen -ls 2>/dev/null || true', {
encoding: 'utf-8',
timeout: 5000
});
// Match: "12345.claudeman-abc12345 (Detached)" or similar
const screenPattern = /(\d+)\.(claudeman-([a-f0-9-]+))/g;
let match;
while ((match = screenPattern.exec(output)) !== null) {
const pid = parseInt(match[1], 10);
const screenName = match[2];
const sessionIdFragment = match[3];
// Check if this screen is already known
let isKnown = false;
for (const screen of this.screens.values()) {
if (screen.screenName === screenName) {
isKnown = true;
break;
}
}
if (!isKnown) {
// Discovered an unknown claudeman screen - adopt it
const sessionId = `restored-${sessionIdFragment}`;
const screen: ScreenSession = {
sessionId,
screenName,
pid,
createdAt: Date.now(),
workingDir: process.cwd(), // Unknown, use current dir
mode: 'claude', // Assume claude mode
attached: false,
name: `Restored: ${screenName}`
};
this.screens.set(sessionId, screen);
discovered.push(sessionId);
console.log(`[ScreenManager] Discovered unknown screen: ${screenName} (PID ${pid})`);
}
}
} catch (err) {
console.error('[ScreenManager] Failed to discover screens:', err);
}
if (dead.length > 0 || discovered.length > 0) {
this.saveScreens(); this.saveScreens();
} }
return { alive, dead }; return { alive, dead, discovered };
} }
// Get process stats for a screen // Get process stats for a screen
+8 -4
View File
@@ -1246,11 +1246,15 @@ export class WebServer extends EventEmitter {
private async restoreScreenSessions(): Promise<void> { private async restoreScreenSessions(): Promise<void> {
try { try {
// Reconcile screens to find which ones are still alive // Reconcile screens to find which ones are still alive (also discovers unknown screens)
const { alive, dead } = await this.screenManager.reconcileScreens(); const { alive, dead, discovered } = await this.screenManager.reconcileScreens();
if (alive.length > 0) { if (discovered.length > 0) {
console.log(`[Server] Found ${alive.length} alive screen session(s) from previous run`); console.log(`[Server] Discovered ${discovered.length} unknown screen session(s)`);
}
if (alive.length > 0 || discovered.length > 0) {
console.log(`[Server] Found ${alive.length + discovered.length} alive screen session(s) from previous run`);
// For each alive screen, create a Session object if it doesn't exist // For each alive screen, create a Session object if it doesn't exist
const screens = this.screenManager.getScreens(); const screens = this.screenManager.getScreens();