mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-04 14:39:42 +02:00
fix: respawn controller multi-layer detection and cleanup
- Update idle detection from legacy '↵ send' to completion message pattern
("for Xm Xs" time patterns like "Worked for 2m 46s")
- Add confirming_idle state for false positive prevention
- Add completionConfirmMs (5s) and noOutputTimeoutMs (30s) config options
- Add multi-layer detection with confidence scoring (0-100%)
- Fix null pointer error in extractTokenCount with guard clause
- Fix respawn controller not stopping on session cleanup (broadcast respawn:stopped)
- Update tests to use new completion message patterns
- Add detection status UI display (confidence level, waiting state)
- Update CLAUDE.md with new detection documentation
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
@@ -31,31 +31,37 @@ class MockSession extends EventEmitter {
|
||||
this.emit('terminal', data);
|
||||
}
|
||||
|
||||
// Simulate prompt appearing (basic prompt character)
|
||||
// Simulate prompt appearing (basic prompt character) - legacy fallback
|
||||
simulatePrompt(): void {
|
||||
this.emit('terminal', '❯ ');
|
||||
}
|
||||
|
||||
// Simulate ready state with the definitive indicator
|
||||
// Simulate ready state with the definitive indicator - legacy fallback
|
||||
simulateReady(): void {
|
||||
this.emit('terminal', '↵ send');
|
||||
}
|
||||
|
||||
// Simulate completion message (NEW - primary idle detection in Claude Code 2024+)
|
||||
// This pattern triggers the multi-layer detection: "for Xm Xs" indicates work finished
|
||||
simulateCompletionMessage(): void {
|
||||
this.emit('terminal', '✻ Worked for 2m 46s');
|
||||
}
|
||||
|
||||
// Simulate working state
|
||||
simulateWorking(): void {
|
||||
this.emit('terminal', 'Thinking... ⠋');
|
||||
}
|
||||
|
||||
// Simulate clear completion (followed by ready indicator)
|
||||
// Simulate clear completion (followed by completion message)
|
||||
simulateClearComplete(): void {
|
||||
this.emit('terminal', 'conversation cleared');
|
||||
setTimeout(() => this.simulateReady(), 50);
|
||||
setTimeout(() => this.simulateCompletionMessage(), 50);
|
||||
}
|
||||
|
||||
// Simulate init completion (followed by ready indicator)
|
||||
// Simulate init completion (followed by completion message)
|
||||
simulateInitComplete(): void {
|
||||
this.emit('terminal', 'Analyzing CLAUDE.md...');
|
||||
setTimeout(() => this.simulateReady(), 100);
|
||||
setTimeout(() => this.simulateCompletionMessage(), 100);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -68,6 +74,8 @@ describe('RespawnController', () => {
|
||||
controller = new RespawnController(session as unknown as Session, {
|
||||
idleTimeoutMs: 100, // Short timeout for testing
|
||||
interStepDelayMs: 50,
|
||||
completionConfirmMs: 50, // Short confirmation delay for testing
|
||||
noOutputTimeoutMs: 500, // Short fallback timeout for testing
|
||||
});
|
||||
});
|
||||
|
||||
@@ -133,24 +141,24 @@ describe('RespawnController', () => {
|
||||
});
|
||||
|
||||
describe('Idle Detection', () => {
|
||||
it('should detect prompt pattern', async () => {
|
||||
it('should detect completion message pattern', async () => {
|
||||
const logMessages: string[] = [];
|
||||
controller.on('log', (msg) => logMessages.push(msg));
|
||||
|
||||
controller.start();
|
||||
session.simulatePrompt();
|
||||
session.simulateCompletionMessage();
|
||||
|
||||
// Wait for log
|
||||
await new Promise(resolve => setTimeout(resolve, 50));
|
||||
|
||||
const hasPromptLog = logMessages.some(msg => msg.includes('Prompt detected'));
|
||||
expect(hasPromptLog).toBe(true);
|
||||
const hasCompletionLog = logMessages.some(msg => msg.includes('Completion message detected'));
|
||||
expect(hasCompletionLog).toBe(true);
|
||||
});
|
||||
|
||||
it('should detect multiple prompt patterns', () => {
|
||||
it('should detect multiple prompt patterns (legacy fallback)', () => {
|
||||
controller.start();
|
||||
|
||||
// All these should trigger prompt detection
|
||||
// All these should trigger prompt detection (legacy)
|
||||
const promptPatterns = ['❯', '\u276f', '⏵', '> ', 'tokens'];
|
||||
|
||||
for (const pattern of promptPatterns) {
|
||||
@@ -163,9 +171,9 @@ describe('RespawnController', () => {
|
||||
|
||||
it('should detect working patterns and clear prompt state', () => {
|
||||
controller.start();
|
||||
session.simulatePrompt();
|
||||
session.simulateCompletionMessage();
|
||||
|
||||
// Simulate working - should clear prompt detected
|
||||
// Simulate working - should clear completion state and cancel confirmation
|
||||
session.simulateWorking();
|
||||
|
||||
const status = controller.getStatus();
|
||||
@@ -175,16 +183,16 @@ describe('RespawnController', () => {
|
||||
});
|
||||
|
||||
describe('Respawn Cycle', () => {
|
||||
it('should start cycle when idle timeout fires', async () => {
|
||||
it('should start cycle when completion message detected and confirmed', async () => {
|
||||
let cycleStarted = false;
|
||||
controller.on('respawnCycleStarted', () => {
|
||||
cycleStarted = true;
|
||||
});
|
||||
|
||||
controller.start();
|
||||
session.simulatePrompt();
|
||||
session.simulateCompletionMessage();
|
||||
|
||||
// Wait for idle timeout
|
||||
// Wait for completion confirmation (completionConfirmMs=50) + processing
|
||||
await new Promise(resolve => setTimeout(resolve, 200));
|
||||
|
||||
expect(cycleStarted).toBe(true);
|
||||
@@ -198,9 +206,9 @@ describe('RespawnController', () => {
|
||||
});
|
||||
|
||||
controller.start();
|
||||
session.simulatePrompt();
|
||||
session.simulateCompletionMessage();
|
||||
|
||||
// Wait for idle timeout and step
|
||||
// Wait for completion confirmation + step delay
|
||||
await new Promise(resolve => setTimeout(resolve, 300));
|
||||
|
||||
expect(stepSent).toBe('update');
|
||||
@@ -213,12 +221,12 @@ describe('RespawnController', () => {
|
||||
controller.on('stateChanged', (state) => states.push(state));
|
||||
|
||||
controller.start();
|
||||
session.simulatePrompt();
|
||||
session.simulateCompletionMessage();
|
||||
|
||||
// Wait for initial state change
|
||||
// Wait for state transitions
|
||||
await new Promise(resolve => setTimeout(resolve, 200));
|
||||
|
||||
// Should have transitioned through multiple states
|
||||
// Should have transitioned through multiple states (watching -> confirming_idle -> sending_update)
|
||||
expect(states).toContain('watching');
|
||||
expect(states.length).toBeGreaterThan(1);
|
||||
});
|
||||
@@ -604,6 +612,8 @@ describe('RespawnController State Transitions', () => {
|
||||
controller = new RespawnController(session as unknown as Session, {
|
||||
idleTimeoutMs: 50,
|
||||
interStepDelayMs: 20,
|
||||
completionConfirmMs: 50, // Short confirmation for testing
|
||||
noOutputTimeoutMs: 300, // Short fallback for testing
|
||||
});
|
||||
});
|
||||
|
||||
@@ -616,7 +626,7 @@ describe('RespawnController State Transitions', () => {
|
||||
controller.on('stateChanged', (state) => stateHistory.push(state));
|
||||
|
||||
controller.start();
|
||||
session.simulatePrompt();
|
||||
session.simulateCompletionMessage();
|
||||
|
||||
await new Promise(resolve => setTimeout(resolve, 200));
|
||||
|
||||
@@ -626,7 +636,7 @@ describe('RespawnController State Transitions', () => {
|
||||
|
||||
it('should handle stop during state transition', async () => {
|
||||
controller.start();
|
||||
session.simulatePrompt();
|
||||
session.simulateCompletionMessage();
|
||||
|
||||
// Wait a bit then stop during potential transition
|
||||
await new Promise(resolve => setTimeout(resolve, 30));
|
||||
@@ -642,7 +652,7 @@ describe('RespawnController State Transitions', () => {
|
||||
});
|
||||
|
||||
controller.start();
|
||||
session.simulateReady();
|
||||
session.simulateCompletionMessage();
|
||||
|
||||
await new Promise(resolve => setTimeout(resolve, 500));
|
||||
|
||||
@@ -651,41 +661,41 @@ describe('RespawnController State Transitions', () => {
|
||||
controller.stop();
|
||||
});
|
||||
|
||||
it('should handle multiple consecutive prompt detections', async () => {
|
||||
let promptCount = 0;
|
||||
it('should handle multiple consecutive completion messages', async () => {
|
||||
let completionCount = 0;
|
||||
controller.on('log', (msg) => {
|
||||
if (msg.includes('Prompt detected')) promptCount++;
|
||||
if (msg.includes('Completion message detected')) completionCount++;
|
||||
});
|
||||
|
||||
controller.start();
|
||||
|
||||
// Send multiple prompts rapidly
|
||||
session.simulatePrompt();
|
||||
session.simulatePrompt();
|
||||
session.simulatePrompt();
|
||||
// Send multiple completion messages rapidly
|
||||
session.simulateCompletionMessage();
|
||||
session.simulateCompletionMessage();
|
||||
session.simulateCompletionMessage();
|
||||
|
||||
await new Promise(resolve => setTimeout(resolve, 100));
|
||||
|
||||
// Should detect prompts
|
||||
expect(promptCount).toBeGreaterThan(0);
|
||||
// Should detect completion messages
|
||||
expect(completionCount).toBeGreaterThan(0);
|
||||
});
|
||||
|
||||
it('should handle working state interrupting idle timeout', async () => {
|
||||
it('should handle working state interrupting idle confirmation', async () => {
|
||||
let cycleStarted = false;
|
||||
controller.on('respawnCycleStarted', () => {
|
||||
cycleStarted = true;
|
||||
});
|
||||
|
||||
controller.start();
|
||||
session.simulatePrompt();
|
||||
session.simulateCompletionMessage();
|
||||
|
||||
// Before idle timeout fires, start working
|
||||
// Before confirmation timer fires, start working
|
||||
await new Promise(resolve => setTimeout(resolve, 20));
|
||||
session.simulateWorking();
|
||||
|
||||
await new Promise(resolve => setTimeout(resolve, 100));
|
||||
|
||||
// Cycle may not have started due to working state
|
||||
// Cycle should not have started due to working state canceling confirmation
|
||||
expect(controller.getStatus().workingDetected).toBe(true);
|
||||
});
|
||||
});
|
||||
@@ -802,14 +812,16 @@ describe('RespawnController Edge Cases', () => {
|
||||
const controller = new RespawnController(session as unknown as Session, {
|
||||
idleTimeoutMs: 30,
|
||||
interStepDelayMs: 10,
|
||||
completionConfirmMs: 30, // Short confirmation for testing
|
||||
noOutputTimeoutMs: 200, // Short fallback for testing
|
||||
});
|
||||
|
||||
expect(controller.currentCycle).toBe(0);
|
||||
|
||||
controller.start();
|
||||
session.simulateReady();
|
||||
session.simulateCompletionMessage();
|
||||
|
||||
await new Promise(resolve => setTimeout(resolve, 150));
|
||||
await new Promise(resolve => setTimeout(resolve, 200));
|
||||
|
||||
expect(controller.currentCycle).toBeGreaterThan(0);
|
||||
controller.stop();
|
||||
|
||||
+56
-117
@@ -3,11 +3,15 @@
|
||||
*
|
||||
* Provides:
|
||||
* - Screen session concurrency limiter (max 10)
|
||||
* - Orphaned Claude/screen process cleanup
|
||||
* - Tracked resource cleanup (only kills what tests create)
|
||||
* - Global beforeAll/afterAll hooks
|
||||
*
|
||||
* SAFETY: This setup ONLY cleans up resources that the test suite itself creates.
|
||||
* It will NEVER kill Claude processes or screens that weren't spawned by tests.
|
||||
* This makes it safe to run tests from within a Claudeman-managed session.
|
||||
*/
|
||||
|
||||
import { execSync, exec } from 'node:child_process';
|
||||
import { execSync } from 'node:child_process';
|
||||
import { beforeAll, afterAll, afterEach } from 'vitest';
|
||||
|
||||
/** Maximum concurrent screen sessions allowed during tests */
|
||||
@@ -16,69 +20,32 @@ const MAX_CONCURRENT_SCREENS = 10;
|
||||
/** Track active screen sessions created during tests */
|
||||
const activeTestScreens = new Set<string>();
|
||||
|
||||
/** Track Claude PIDs spawned by tests (for cleanup) */
|
||||
const activeTestClaudePids = new Set<number>();
|
||||
|
||||
/** Semaphore for controlling concurrent screen creation */
|
||||
let currentScreenCount = 0;
|
||||
const screenWaiters: Array<() => void> = [];
|
||||
|
||||
/**
|
||||
* Get list of claudeman screen sessions
|
||||
* Kill only the screens that tests have registered via registerTestScreen()
|
||||
*/
|
||||
function getClaudemanScreens(): string[] {
|
||||
try {
|
||||
const output = execSync('screen -ls 2>/dev/null || true', { encoding: 'utf-8' });
|
||||
const lines = output.split('\n');
|
||||
const screens: string[] = [];
|
||||
for (const line of lines) {
|
||||
const match = line.match(/\d+\.(claudeman-[^\s]+)/);
|
||||
if (match) {
|
||||
screens.push(match[1]);
|
||||
}
|
||||
}
|
||||
return screens;
|
||||
} catch {
|
||||
return [];
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Get list of Claude CLI processes
|
||||
*/
|
||||
function getClaudeProcesses(): number[] {
|
||||
try {
|
||||
const output = execSync('pgrep -f "claude.*--dangerously-skip-permissions" 2>/dev/null || true', {
|
||||
encoding: 'utf-8',
|
||||
});
|
||||
return output
|
||||
.trim()
|
||||
.split('\n')
|
||||
.filter(Boolean)
|
||||
.map(pid => parseInt(pid, 10))
|
||||
.filter(pid => !isNaN(pid));
|
||||
} catch {
|
||||
return [];
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Kill orphaned claudeman screen sessions
|
||||
*/
|
||||
function killOrphanedScreens(): void {
|
||||
const screens = getClaudemanScreens();
|
||||
for (const screenName of screens) {
|
||||
function killTrackedTestScreens(): void {
|
||||
for (const screenName of activeTestScreens) {
|
||||
try {
|
||||
execSync(`screen -S ${screenName} -X quit 2>/dev/null || true`, { encoding: 'utf-8' });
|
||||
} catch {
|
||||
// Ignore errors
|
||||
}
|
||||
}
|
||||
activeTestScreens.clear();
|
||||
}
|
||||
|
||||
/**
|
||||
* Kill orphaned Claude CLI processes
|
||||
* Kill only the Claude processes that tests have registered via registerTestClaudePid()
|
||||
*/
|
||||
function killOrphanedClaudeProcesses(): void {
|
||||
const pids = getClaudeProcesses();
|
||||
for (const pid of pids) {
|
||||
function killTrackedTestClaudeProcesses(): void {
|
||||
for (const pid of activeTestClaudePids) {
|
||||
try {
|
||||
process.kill(pid, 'SIGTERM');
|
||||
} catch {
|
||||
@@ -87,16 +54,19 @@ function killOrphanedClaudeProcesses(): void {
|
||||
}
|
||||
|
||||
// Wait a bit, then SIGKILL any remaining
|
||||
if (pids.length > 0) {
|
||||
if (activeTestClaudePids.size > 0) {
|
||||
setTimeout(() => {
|
||||
for (const pid of pids) {
|
||||
for (const pid of activeTestClaudePids) {
|
||||
try {
|
||||
process.kill(pid, 'SIGKILL');
|
||||
} catch {
|
||||
// Process may already be gone
|
||||
}
|
||||
}
|
||||
activeTestClaudePids.clear();
|
||||
}, 500);
|
||||
} else {
|
||||
activeTestClaudePids.clear();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -143,6 +113,20 @@ export function unregisterTestScreen(screenName: string): void {
|
||||
activeTestScreens.delete(screenName);
|
||||
}
|
||||
|
||||
/**
|
||||
* Register a Claude PID for tracking (so it gets cleaned up after tests)
|
||||
*/
|
||||
export function registerTestClaudePid(pid: number): void {
|
||||
activeTestClaudePids.add(pid);
|
||||
}
|
||||
|
||||
/**
|
||||
* Unregister a Claude PID
|
||||
*/
|
||||
export function unregisterTestClaudePid(pid: number): void {
|
||||
activeTestClaudePids.delete(pid);
|
||||
}
|
||||
|
||||
/**
|
||||
* Get current screen count for debugging
|
||||
*/
|
||||
@@ -155,21 +139,15 @@ export function getScreenStats(): { current: number; max: number; waiting: numbe
|
||||
}
|
||||
|
||||
/**
|
||||
* Force cleanup all test screens (emergency cleanup)
|
||||
* Force cleanup all test-created resources (emergency cleanup)
|
||||
* Only kills resources that tests have registered - never kills external processes
|
||||
*/
|
||||
export function forceCleanupAllScreens(): void {
|
||||
export function forceCleanupAllTestResources(): void {
|
||||
// Kill all tracked test screens
|
||||
for (const screenName of activeTestScreens) {
|
||||
try {
|
||||
execSync(`screen -S ${screenName} -X quit 2>/dev/null || true`);
|
||||
} catch {
|
||||
// Ignore
|
||||
}
|
||||
}
|
||||
activeTestScreens.clear();
|
||||
killTrackedTestScreens();
|
||||
|
||||
// Also kill any orphaned claudeman screens
|
||||
killOrphanedScreens();
|
||||
// Kill all tracked Claude processes
|
||||
killTrackedTestClaudeProcesses();
|
||||
|
||||
// Reset semaphore
|
||||
currentScreenCount = 0;
|
||||
@@ -181,64 +159,27 @@ export function forceCleanupAllScreens(): void {
|
||||
// =============================================================================
|
||||
|
||||
beforeAll(async () => {
|
||||
// Clean up any orphaned processes from previous test runs
|
||||
console.log('[Test Setup] Cleaning up orphaned processes...');
|
||||
|
||||
killOrphanedScreens();
|
||||
killOrphanedClaudeProcesses();
|
||||
|
||||
// Wait for cleanup to complete
|
||||
await new Promise(resolve => setTimeout(resolve, 1000));
|
||||
|
||||
const remainingScreens = getClaudemanScreens();
|
||||
const remainingClaude = getClaudeProcesses();
|
||||
|
||||
if (remainingScreens.length > 0) {
|
||||
console.log(`[Test Setup] Warning: ${remainingScreens.length} screen sessions still exist`);
|
||||
}
|
||||
if (remainingClaude.length > 0) {
|
||||
console.log(`[Test Setup] Warning: ${remainingClaude.length} Claude processes still exist`);
|
||||
}
|
||||
|
||||
console.log('[Test Setup] Cleanup complete, starting tests...');
|
||||
// Initialize test environment
|
||||
console.log('[Test Setup] Initializing test environment...');
|
||||
console.log('[Test Setup] Only test-created resources will be cleaned up (safe for Claudeman sessions)');
|
||||
console.log('[Test Setup] Starting tests...');
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
console.log('[Test Setup] Final cleanup...');
|
||||
console.log('[Test Setup] Final cleanup of test-created resources...');
|
||||
|
||||
// Force cleanup all screens
|
||||
forceCleanupAllScreens();
|
||||
|
||||
// Kill any remaining Claude processes
|
||||
killOrphanedClaudeProcesses();
|
||||
// Only cleanup resources that tests have registered
|
||||
forceCleanupAllTestResources();
|
||||
|
||||
// Wait for cleanup
|
||||
await new Promise(resolve => setTimeout(resolve, 1000));
|
||||
await new Promise(resolve => setTimeout(resolve, 500));
|
||||
|
||||
const remainingScreens = getClaudemanScreens();
|
||||
const remainingClaude = getClaudeProcesses();
|
||||
|
||||
if (remainingScreens.length > 0) {
|
||||
console.warn(`[Test Setup] Warning: ${remainingScreens.length} orphaned screens after tests`);
|
||||
// Force kill them
|
||||
for (const screen of remainingScreens) {
|
||||
try {
|
||||
execSync(`screen -S ${screen} -X quit 2>/dev/null || true`);
|
||||
} catch {
|
||||
// Ignore
|
||||
}
|
||||
}
|
||||
// Report any tracked resources that weren't cleaned up
|
||||
if (activeTestScreens.size > 0) {
|
||||
console.warn(`[Test Setup] Warning: ${activeTestScreens.size} test screens weren't properly unregistered`);
|
||||
}
|
||||
|
||||
if (remainingClaude.length > 0) {
|
||||
console.warn(`[Test Setup] Warning: ${remainingClaude.length} orphaned Claude processes after tests`);
|
||||
for (const pid of remainingClaude) {
|
||||
try {
|
||||
process.kill(pid, 'SIGKILL');
|
||||
} catch {
|
||||
// Ignore
|
||||
}
|
||||
}
|
||||
if (activeTestClaudePids.size > 0) {
|
||||
console.warn(`[Test Setup] Warning: ${activeTestClaudePids.size} test Claude PIDs weren't properly unregistered`);
|
||||
}
|
||||
|
||||
console.log('[Test Setup] Final cleanup complete');
|
||||
@@ -246,9 +187,7 @@ afterAll(async () => {
|
||||
|
||||
// Export utilities for tests that need them
|
||||
export {
|
||||
getClaudemanScreens,
|
||||
getClaudeProcesses,
|
||||
killOrphanedScreens,
|
||||
killOrphanedClaudeProcesses,
|
||||
killTrackedTestScreens,
|
||||
killTrackedTestClaudeProcesses,
|
||||
MAX_CONCURRENT_SCREENS,
|
||||
};
|
||||
|
||||
Reference in New Issue
Block a user