mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-09-30 12:39:42 +02:00
fix: use temp file for plan checker prompt to avoid E2BIG + fix cancel race condition
The ai-plan-checker.ts was passing the prompt directly as a shell argument, which can cause E2BIG errors when the terminal buffer is large (8KB+). This fix applies the same temp file approach already used in ai-idle-checker.ts: - Write prompt to a temp file instead of passing as shell argument - Pipe the file to claude via stdin: `cat prompt.txt | claude -p ...` - Clean up prompt file after check completes Also fixes a race condition in the cancel() method of both AI checkers where the poll timer could fire between setting checkCancelled and clearing timers. Now timers are cleared before resolving the promise to prevent this race. Includes test utilities and analysis documents for the respawn controller created by other agents: - test/respawn-test-utils.ts - MockSession, MockAiIdleChecker utilities - test/respawn-analysis.md - Code analysis and issue identification - test/respawn-scenarios.md - Test scenario documentation - test/respawn-test-plan.md - Testing architecture documentation Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
arkon
@@ -17,7 +17,7 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co
|
||||
|
||||
Claudeman is a Claude Code session manager with a web interface and autonomous Ralph Loop. It spawns Claude CLI processes via PTY, streams output in real-time via SSE, and supports scheduled/timed runs.
|
||||
|
||||
**Version**: 0.1345
|
||||
**Version**: 0.1346
|
||||
|
||||
**Tech Stack**: TypeScript (ES2022/NodeNext, strict mode), Node.js, Fastify, Server-Sent Events, node-pty
|
||||
|
||||
|
||||
+1
-1
@@ -1,6 +1,6 @@
|
||||
{
|
||||
"name": "claudeman",
|
||||
"version": "0.1345",
|
||||
"version": "0.1346",
|
||||
"description": "The missing control plane for Claude Code - run 20 autonomous agents with real-time monitoring and session persistence",
|
||||
"type": "module",
|
||||
"main": "dist/index.js",
|
||||
|
||||
@@ -268,13 +268,16 @@ export class AiIdleChecker extends EventEmitter {
|
||||
this.log('Cancelling AI check');
|
||||
this.checkCancelled = true;
|
||||
|
||||
// Resolve the pending promise before cleanup
|
||||
// Clear poll/timeout timers first to prevent race condition where
|
||||
// the poll timer fires between setting checkCancelled and cleanup
|
||||
this.cleanupCheck();
|
||||
|
||||
// Resolve the pending promise after cleanup
|
||||
if (this.checkResolve) {
|
||||
this.checkResolve({ verdict: 'ERROR', reasoning: 'Cancelled', durationMs: Date.now() - this.checkStartTime });
|
||||
this.checkResolve = null;
|
||||
}
|
||||
|
||||
this.cleanupCheck();
|
||||
this._status = 'ready';
|
||||
}
|
||||
|
||||
|
||||
+28
-9
@@ -152,6 +152,7 @@ export class AiPlanChecker extends EventEmitter {
|
||||
// Active check state
|
||||
private checkScreenName: string | null = null;
|
||||
private checkTempFile: string | null = null;
|
||||
private checkPromptFile: string | null = null;
|
||||
private checkPollTimer: NodeJS.Timeout | null = null;
|
||||
private checkTimeoutTimer: NodeJS.Timeout | null = null;
|
||||
private checkStartTime: number = 0;
|
||||
@@ -276,13 +277,16 @@ export class AiPlanChecker extends EventEmitter {
|
||||
this.log('Cancelling AI plan check');
|
||||
this.checkCancelled = true;
|
||||
|
||||
// Resolve the pending promise before cleanup
|
||||
// Clear poll/timeout timers first to prevent race condition where
|
||||
// the poll timer fires between setting checkCancelled and cleanup
|
||||
this.cleanupCheck();
|
||||
|
||||
// Resolve the pending promise after cleanup
|
||||
if (this.checkResolve) {
|
||||
this.checkResolve({ verdict: 'ERROR', reasoning: 'Cancelled', durationMs: Date.now() - this.checkStartTime });
|
||||
this.checkResolve = null;
|
||||
}
|
||||
|
||||
this.cleanupCheck();
|
||||
this._status = 'ready';
|
||||
}
|
||||
|
||||
@@ -329,21 +333,25 @@ export class AiPlanChecker extends EventEmitter {
|
||||
// Build the prompt
|
||||
const prompt = AI_PLAN_CHECK_PROMPT.replace('{TERMINAL_BUFFER}', trimmed);
|
||||
|
||||
// Generate temp file and screen name
|
||||
// Generate temp files and screen name
|
||||
const shortId = this.sessionId.slice(0, 8);
|
||||
const timestamp = Date.now();
|
||||
this.checkTempFile = join(tmpdir(), `claudeman-plancheck-${shortId}-${timestamp}.txt`);
|
||||
this.checkPromptFile = join(tmpdir(), `claudeman-plancheck-prompt-${shortId}-${timestamp}.txt`);
|
||||
this.checkScreenName = `claudeman-plancheck-${shortId}`;
|
||||
|
||||
// Ensure temp file exists (empty) so we can poll it
|
||||
// Ensure output temp file exists (empty) so we can poll it
|
||||
writeFileSync(this.checkTempFile, '');
|
||||
|
||||
// Build the command - escape the prompt for shell
|
||||
const escapedPrompt = prompt.replace(/'/g, "'\\''");
|
||||
// Write prompt to file to avoid E2BIG error (argument list too long)
|
||||
// The prompt can be 8KB+ which exceeds shell argument limits
|
||||
writeFileSync(this.checkPromptFile, prompt);
|
||||
|
||||
// Build the command - read prompt from file via stdin to avoid argument size limits
|
||||
const modelArg = `--model ${this.config.model}`;
|
||||
const augmentedPath = getAugmentedPath();
|
||||
const claudeCmd = `claude -p ${modelArg} --output-format text '${escapedPrompt}'`;
|
||||
const fullCmd = `export PATH="${augmentedPath}"; ${claudeCmd} > "${this.checkTempFile}" 2>&1; echo "${DONE_MARKER}" >> "${this.checkTempFile}"`;
|
||||
const claudeCmd = `cat "${this.checkPromptFile}" | claude -p ${modelArg} --output-format text`;
|
||||
const fullCmd = `export PATH="${augmentedPath}"; ${claudeCmd} > "${this.checkTempFile}" 2>&1; echo "${DONE_MARKER}" >> "${this.checkTempFile}"; rm -f "${this.checkPromptFile}"`;
|
||||
|
||||
// Spawn screen
|
||||
try {
|
||||
@@ -447,7 +455,7 @@ export class AiPlanChecker extends EventEmitter {
|
||||
this.checkScreenName = null;
|
||||
}
|
||||
|
||||
// Delete temp file
|
||||
// Delete temp files
|
||||
if (this.checkTempFile) {
|
||||
try {
|
||||
if (existsSync(this.checkTempFile)) {
|
||||
@@ -458,6 +466,17 @@ export class AiPlanChecker extends EventEmitter {
|
||||
}
|
||||
this.checkTempFile = null;
|
||||
}
|
||||
|
||||
if (this.checkPromptFile) {
|
||||
try {
|
||||
if (existsSync(this.checkPromptFile)) {
|
||||
unlinkSync(this.checkPromptFile);
|
||||
}
|
||||
} catch {
|
||||
// Best effort cleanup
|
||||
}
|
||||
this.checkPromptFile = null;
|
||||
}
|
||||
}
|
||||
|
||||
private handleError(errorMsg: string): void {
|
||||
|
||||
@@ -0,0 +1,295 @@
|
||||
# Respawn Controller Test Analysis
|
||||
|
||||
**Date**: 2026-01-25
|
||||
**Analyzed Files**:
|
||||
- `/home/arkon/default/claudeman/src/respawn-controller.ts`
|
||||
- `/home/arkon/default/claudeman/src/ai-idle-checker.ts`
|
||||
- `/home/arkon/default/claudeman/src/ai-plan-checker.ts`
|
||||
- `/home/arkon/default/claudeman/test/respawn-controller.test.ts`
|
||||
|
||||
---
|
||||
|
||||
## 1. Test Execution Results
|
||||
|
||||
### Summary
|
||||
- **Total Tests**: 91
|
||||
- **Passed**: 91
|
||||
- **Failed**: 0
|
||||
- **Duration**: ~10 seconds
|
||||
|
||||
### Type Check Results
|
||||
- **Type Errors**: 0
|
||||
|
||||
All tests pass and the codebase is type-safe.
|
||||
|
||||
---
|
||||
|
||||
## 2. Code Coverage Analysis
|
||||
|
||||
### Coverage Summary
|
||||
|
||||
| File | Statements | Branches | Functions | Lines |
|
||||
|------|------------|----------|-----------|-------|
|
||||
| respawn-controller.ts | 62.51% | 59.85% | 74.48% | 62.37% |
|
||||
| ai-idle-checker.ts | 68.91% | 46.57% | 75% | 69.31% |
|
||||
| ai-plan-checker.ts | 57.52% | 37.68% | 58.33% | 57.69% |
|
||||
|
||||
### Uncovered Code in respawn-controller.ts
|
||||
|
||||
The following areas lack test coverage:
|
||||
|
||||
1. **Lines 2077-2151**: `sendClear()`, `sendInit()`, `completeCycle()`
|
||||
- These functions execute during the full respawn cycle
|
||||
- Tests trigger cycles but don't mock session responses to complete them
|
||||
|
||||
2. **Line 2162**: `checkIdleAndMaybeStart()`
|
||||
- Called when resuming from pause while already idle
|
||||
- Only tested superficially with `resume()` call
|
||||
|
||||
3. **Lines 2100-2103**: Clear fallback timer path when `sendInit` is false
|
||||
- Edge case where `/clear` completes via fallback but init is disabled
|
||||
|
||||
---
|
||||
|
||||
## 3. Potential Bugs and Issues
|
||||
|
||||
### Issue 1: E2BIG Error in ai-plan-checker.ts (HIGH)
|
||||
|
||||
**File**: `/home/arkon/default/claudeman/src/ai-plan-checker.ts`
|
||||
**Lines**: 341-346
|
||||
**Severity**: HIGH
|
||||
|
||||
**Description**: Unlike `ai-idle-checker.ts`, the plan checker passes the prompt directly as a shell argument rather than via a temp file. This can cause `E2BIG` (argument list too long) errors when the terminal buffer is large.
|
||||
|
||||
**Code**:
|
||||
```typescript
|
||||
// ai-plan-checker.ts - PROBLEMATIC
|
||||
const escapedPrompt = prompt.replace(/'/g, "'\\''");
|
||||
const claudeCmd = `claude -p ${modelArg} --output-format text '${escapedPrompt}'`;
|
||||
|
||||
// ai-idle-checker.ts - CORRECT
|
||||
writeFileSync(this.checkPromptFile, prompt);
|
||||
const claudeCmd = `cat "${this.checkPromptFile}" | claude -p ${modelArg} --output-format text`;
|
||||
```
|
||||
|
||||
**Impact**: With `maxContextChars: 8000`, the prompt can easily exceed shell argument limits (~128KB on most systems), causing the AI plan check to fail with a cryptic error.
|
||||
|
||||
**Suggested Fix**: Modify `ai-plan-checker.ts` to use the same temp file approach as `ai-idle-checker.ts`:
|
||||
1. Add `checkPromptFile` instance variable
|
||||
2. Write prompt to temp file
|
||||
3. Use `cat | claude -p` pattern
|
||||
4. Clean up temp file in `cleanupCheck()`
|
||||
|
||||
---
|
||||
|
||||
### Issue 2: Missing Prompt File Cleanup on Timeout (MEDIUM)
|
||||
|
||||
**File**: `/home/arkon/default/claudeman/src/ai-idle-checker.ts`
|
||||
**Lines**: 392-398
|
||||
**Severity**: MEDIUM
|
||||
|
||||
**Description**: When the AI check times out, the `cleanupCheck()` function is called via `finally`. However, the prompt file deletion in `cleanupCheck()` may fail silently if the file is still being read by the `cat` command in the spawned screen.
|
||||
|
||||
**Impact**: Temp files may accumulate in `/tmp` if checks frequently timeout.
|
||||
|
||||
**Suggested Fix**: Add a small delay before file deletion or use a unique timestamp-based naming scheme that guarantees no collisions (already partially implemented but the shell command `rm -f` in the fullCmd also handles this).
|
||||
|
||||
---
|
||||
|
||||
### Issue 3: Race Condition in cancel() with Promise Resolution (MEDIUM)
|
||||
|
||||
**File**: `/home/arkon/default/claudeman/src/ai-idle-checker.ts`
|
||||
**Lines**: 265-279
|
||||
**Severity**: MEDIUM
|
||||
|
||||
**Description**: The `cancel()` method resolves the pending promise and then calls `cleanupCheck()`. However, the interval poll timer might fire between these two operations and attempt to resolve an already-resolved promise.
|
||||
|
||||
**Code**:
|
||||
```typescript
|
||||
cancel(): void {
|
||||
if (this._status !== 'checking') return;
|
||||
this.checkCancelled = true;
|
||||
// Race: poll timer might fire here
|
||||
if (this.checkResolve) {
|
||||
this.checkResolve({ verdict: 'ERROR', ... });
|
||||
this.checkResolve = null;
|
||||
}
|
||||
this.cleanupCheck(); // This clears the poll timer
|
||||
this._status = 'ready';
|
||||
}
|
||||
```
|
||||
|
||||
**Impact**: Could theoretically cause a double-resolve, though the guard `if (this.checkCancelled)` in the poll handler mitigates this.
|
||||
|
||||
**Suggested Fix**: Set `checkCancelled = true` and call `cleanupCheck()` (which clears the poll timer) before resolving the promise.
|
||||
|
||||
---
|
||||
|
||||
### Issue 4: Stale Screen Name Collision (LOW)
|
||||
|
||||
**File**: `/home/arkon/default/claudeman/src/ai-idle-checker.ts`, `ai-plan-checker.ts`
|
||||
**Lines**: 326-329 (idle), 333-336 (plan)
|
||||
**Severity**: LOW
|
||||
|
||||
**Description**: Screen names are generated using only `sessionId.slice(0, 8)`, without a unique suffix. While there's a `screen -X quit` to kill leftover screens, two rapid checks could conflict.
|
||||
|
||||
**Code**:
|
||||
```typescript
|
||||
this.checkScreenName = `claudeman-aicheck-${shortId}`;
|
||||
```
|
||||
|
||||
**Impact**: Very unlikely in practice since checks have cooldowns, but theoretically possible.
|
||||
|
||||
**Suggested Fix**: Already partially mitigated by the `timestamp` in temp file names. Could add timestamp to screen name too:
|
||||
```typescript
|
||||
this.checkScreenName = `claudeman-aicheck-${shortId}-${timestamp}`;
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
### Issue 5: DetectionStatus Calculation During ai_checking State (LOW)
|
||||
|
||||
**File**: `/home/arkon/default/claudeman/src/respawn-controller.ts`
|
||||
**Lines**: 759-762
|
||||
**Severity**: LOW
|
||||
|
||||
**Description**: The `outputSilent` calculation uses `config.completionConfirmMs`, but when in `ai_checking` state, the output silence threshold should conceptually be the AI check timeout, not the completion confirm time.
|
||||
|
||||
**Impact**: UI display may show incorrect "silence" status during AI check.
|
||||
|
||||
---
|
||||
|
||||
## 4. Coverage Gaps
|
||||
|
||||
### Functions Not Fully Tested
|
||||
|
||||
1. **`sendClear()`** - Never reaches execution in tests because the cycle is cut short
|
||||
2. **`sendInit()`** - Same as above
|
||||
3. **`completeCycle()`** - Tests don't allow cycles to complete fully
|
||||
4. **`checkClearComplete()`** - Requires mocking prompt detection after /clear
|
||||
5. **`checkInitComplete()`** - Requires mocking init completion flow
|
||||
6. **`sendKickstart()`** - Not tested at all
|
||||
7. **`checkKickstartComplete()`** - Not tested
|
||||
8. **`startMonitoringInit()`** - Not tested
|
||||
9. **`checkMonitoringInitIdle()`** - Not tested
|
||||
|
||||
### Branches Not Tested
|
||||
|
||||
1. `/clear` fallback timer completing (line 2095-2105)
|
||||
2. `sendInit: false` branch in various locations
|
||||
3. Kickstart prompt flow when `/init` doesn't trigger work
|
||||
4. AI checker returning `WORKING` verdict (only test with timeout)
|
||||
5. AI plan checker `PLAN_MODE` verdict (real check, not just pre-filter)
|
||||
|
||||
### Edge Cases Not Tested
|
||||
|
||||
1. Multiple rapid AI checks before cooldown
|
||||
2. AI checker disabled mid-check
|
||||
3. Session terminal buffer being `undefined` on start
|
||||
4. Buffer trimming behavior at MAX_RESPAWN_BUFFER_SIZE
|
||||
5. `signalElicitation()` called multiple times
|
||||
|
||||
---
|
||||
|
||||
## 5. Timing and Race Condition Analysis
|
||||
|
||||
### Potential Timing Issues
|
||||
|
||||
1. **Pre-filter timer vs AI check**: If pre-filter fires while an AI check is already running, it correctly skips (line 1613).
|
||||
|
||||
2. **Completion confirm during AI check**: Output during AI check cancels it (line 1173-1188), which is correct.
|
||||
|
||||
3. **Auto-accept during respawn cycle**: Correctly guards against non-watching state (line 1741).
|
||||
|
||||
4. **Step confirm timer**: Uses same `completionConfirmMs` as idle confirm, which may be too short for complex operations.
|
||||
|
||||
### Timer Cleanup
|
||||
|
||||
All timers appear to be properly cleaned up in `clearTimers()`:
|
||||
- `idleTimer`
|
||||
- `stepTimer`
|
||||
- `clearFallbackTimer`
|
||||
- `completionConfirmTimer`
|
||||
- `stepConfirmTimer`
|
||||
- `autoAcceptTimer`
|
||||
- `preFilterTimer`
|
||||
- `noOutputTimer`
|
||||
- `detectionUpdateTimer` (interval)
|
||||
|
||||
---
|
||||
|
||||
## 6. Recommendations (Prioritized)
|
||||
|
||||
### Critical (Should Fix)
|
||||
|
||||
1. **Fix E2BIG vulnerability in ai-plan-checker.ts**
|
||||
- Use temp file for prompt like ai-idle-checker.ts does
|
||||
- Add `checkPromptFile` instance variable
|
||||
- Update `cleanupCheck()` to delete prompt file
|
||||
|
||||
### High Priority
|
||||
|
||||
2. **Add integration tests for full respawn cycle**
|
||||
- Mock session to simulate prompt detection after each step
|
||||
- Test: update -> clear -> init -> back to watching
|
||||
- Test: kickstart prompt when init doesn't trigger work
|
||||
|
||||
3. **Fix cancel() race condition in AI checkers**
|
||||
- Clear poll timer before resolving promise
|
||||
- Or use a mutex/flag check in poll handler
|
||||
|
||||
### Medium Priority
|
||||
|
||||
4. **Add tests for AI checker WORKING verdict**
|
||||
- Mock the screen process to return WORKING
|
||||
- Verify cooldown is started
|
||||
- Verify controller returns to watching
|
||||
|
||||
5. **Add tests for plan checker PLAN_MODE verdict**
|
||||
- Mock the screen process to return PLAN_MODE
|
||||
- Verify Enter is sent
|
||||
- Verify state transitions
|
||||
|
||||
6. **Add timestamp to screen session names**
|
||||
- Prevents potential collisions during rapid operations
|
||||
|
||||
### Low Priority
|
||||
|
||||
7. **Improve coverage of edge cases**
|
||||
- Test `sendInit: false` and `sendClear: false` combinations
|
||||
- Test buffer trimming behavior
|
||||
- Test pause/resume with activity detection
|
||||
|
||||
8. **Consider reducing step confirm timeout**
|
||||
- Currently uses `completionConfirmMs` (10s default)
|
||||
- May want separate config for step confirmation
|
||||
|
||||
---
|
||||
|
||||
## 7. MockSession Limitations
|
||||
|
||||
The current `MockSession` class in tests is simplified and doesn't accurately simulate:
|
||||
|
||||
1. **Delayed responses**: Real sessions have processing time between input and output
|
||||
2. **State persistence**: Real sessions maintain state between commands
|
||||
3. **Screen integration**: Real sessions use GNU screen for persistence
|
||||
4. **Token tracking**: Real sessions parse and track token usage
|
||||
|
||||
Consider adding a more sophisticated mock that can:
|
||||
- Queue delayed responses
|
||||
- Simulate multi-step workflows
|
||||
- Return realistic completion messages with timing
|
||||
|
||||
---
|
||||
|
||||
## 8. Conclusion
|
||||
|
||||
The respawn controller tests are comprehensive for basic functionality but lack coverage for:
|
||||
- Full respawn cycle completion
|
||||
- AI checker verdicts (IDLE/WORKING/PLAN_MODE)
|
||||
- Kickstart functionality
|
||||
- Edge cases and error handling
|
||||
|
||||
The main bug found is in `ai-plan-checker.ts` which can fail with E2BIG errors on large terminal buffers. This should be fixed by using the temp file approach already implemented in `ai-idle-checker.ts`.
|
||||
|
||||
All 91 tests pass, and there are no type errors, indicating a stable codebase but with room for deeper integration testing.
|
||||
File diff suppressed because it is too large
Load Diff
@@ -0,0 +1,482 @@
|
||||
# Respawn Controller Test Plan
|
||||
|
||||
This document describes the testing environment, architecture, and strategies for testing the RespawnController and related AI checker components.
|
||||
|
||||
## Table of Contents
|
||||
|
||||
1. [Test Environment Architecture](#test-environment-architecture)
|
||||
2. [Component Interaction Diagram](#component-interaction-diagram)
|
||||
3. [Mock Components](#mock-components)
|
||||
4. [Port Allocation](#port-allocation)
|
||||
5. [Test Isolation Strategy](#test-isolation-strategy)
|
||||
6. [Cleanup Procedures](#cleanup-procedures)
|
||||
7. [Test Categories](#test-categories)
|
||||
8. [Usage Examples](#usage-examples)
|
||||
|
||||
---
|
||||
|
||||
## Test Environment Architecture
|
||||
|
||||
The RespawnController testing environment uses a layered approach to isolate components and enable deterministic testing without spawning real Claude CLI processes.
|
||||
|
||||
```
|
||||
+------------------------------------------+
|
||||
| Test Runner (Vitest) |
|
||||
+------------------------------------------+
|
||||
| |
|
||||
v v
|
||||
+------------------+ +------------------+
|
||||
| Unit Tests | | Integration Tests|
|
||||
| (No real I/O) | | (Uses ports) |
|
||||
+------------------+ +------------------+
|
||||
| |
|
||||
v v
|
||||
+------------------------------------------+
|
||||
| Test Utilities Layer |
|
||||
| - MockSession |
|
||||
| - MockAiIdleChecker |
|
||||
| - MockAiPlanChecker |
|
||||
| - TimeController |
|
||||
| - State/Event Recorders |
|
||||
+------------------------------------------+
|
||||
|
|
||||
v
|
||||
+------------------------------------------+
|
||||
| Real Components Under Test |
|
||||
| - RespawnController |
|
||||
| - RespawnConfig types |
|
||||
| - State machine logic |
|
||||
+------------------------------------------+
|
||||
```
|
||||
|
||||
### Key Principles
|
||||
|
||||
1. **No Real Claude CLI**: Tests use MockAiIdleChecker and MockAiPlanChecker instead of spawning real Claude processes
|
||||
2. **No Real Screens**: MockSession simulates terminal I/O without GNU screen
|
||||
3. **Deterministic Timing**: Tests can use real timers (short timeouts) or fake timers for precise control
|
||||
4. **Isolated State**: Each test gets fresh instances with no shared state
|
||||
|
||||
---
|
||||
|
||||
## Component Interaction Diagram
|
||||
|
||||
```
|
||||
RespawnController
|
||||
|
|
||||
+-----------------+-----------------+
|
||||
| | |
|
||||
v v v
|
||||
Session AiIdleChecker AiPlanChecker
|
||||
(MockSession) (MockAiIdleChecker) (MockAiPlanChecker)
|
||||
| | |
|
||||
v | |
|
||||
Terminal Events AI Check AI Check
|
||||
(simulateXxx) Verdicts Verdicts
|
||||
| | |
|
||||
+--------+--------+---------+-------+
|
||||
|
|
||||
v
|
||||
State Machine
|
||||
(watching -> ... -> stopped)
|
||||
|
|
||||
v
|
||||
Event Emission
|
||||
(stateChanged, stepSent, etc.)
|
||||
```
|
||||
|
||||
### Data Flow
|
||||
|
||||
1. **Terminal Output Flow**:
|
||||
- `MockSession.simulateTerminalOutput(data)` -> `session.emit('terminal', data)`
|
||||
- RespawnController receives terminal event in `handleTerminalData()`
|
||||
- Pattern detection (completion, working, prompt)
|
||||
- Timer management (start/reset/cancel)
|
||||
|
||||
2. **AI Check Flow**:
|
||||
- Pre-filter conditions met -> `tryStartAiCheck()`
|
||||
- MockAiIdleChecker returns queued or default verdict
|
||||
- Controller processes verdict (IDLE -> start cycle, WORKING -> cooldown)
|
||||
|
||||
3. **State Transition Flow**:
|
||||
- Internal state changes via `setState()`
|
||||
- Events emitted for external monitoring
|
||||
- Timers started/cancelled based on state
|
||||
|
||||
---
|
||||
|
||||
## Mock Components
|
||||
|
||||
### MockSession
|
||||
|
||||
Enhanced session mock that simulates Claude Code terminal behavior.
|
||||
|
||||
**Key Methods**:
|
||||
- `simulateTerminalOutput(data)` - Raw terminal output
|
||||
- `simulateCompletionMessage(duration?)` - "Worked for Xm Xs" pattern
|
||||
- `simulateWorking(text?)` - Spinner/activity indicators
|
||||
- `simulatePlanModePrompt()` - Numbered selection menu
|
||||
- `simulateElicitationDialog()` - AskUserQuestion prompt
|
||||
- `writeBuffer` - Captures all writes for assertions
|
||||
|
||||
**Usage**:
|
||||
```typescript
|
||||
const session = createMockSession();
|
||||
session.simulateCompletionMessage();
|
||||
await waitForState(controller, 'confirming_idle');
|
||||
```
|
||||
|
||||
### MockAiIdleChecker
|
||||
|
||||
Mock for AI idle detection that returns configurable verdicts.
|
||||
|
||||
**Key Features**:
|
||||
- Queue-based result system (FIFO)
|
||||
- Default verdict when queue empty
|
||||
- Cooldown simulation
|
||||
- Error/disabled state simulation
|
||||
|
||||
**Usage**:
|
||||
```typescript
|
||||
const checker = new MockAiIdleChecker('session-id');
|
||||
checker.setNextIdle('Task completed');
|
||||
// or
|
||||
checker.queueResults(
|
||||
{ verdict: 'WORKING', reasoning: 'Still active', durationMs: 100 },
|
||||
{ verdict: 'IDLE', reasoning: 'Now idle', durationMs: 150 }
|
||||
);
|
||||
```
|
||||
|
||||
### MockAiPlanChecker
|
||||
|
||||
Mock for AI plan mode detection.
|
||||
|
||||
**Key Features**:
|
||||
- Same queue/default pattern as MockAiIdleChecker
|
||||
- PLAN_MODE/NOT_PLAN_MODE verdicts
|
||||
- Cooldown after NOT_PLAN_MODE
|
||||
|
||||
**Usage**:
|
||||
```typescript
|
||||
const checker = new MockAiPlanChecker('session-id');
|
||||
checker.setNextPlanMode('Approval prompt detected');
|
||||
```
|
||||
|
||||
### TimeController
|
||||
|
||||
Wrapper around Vitest's fake timers for deterministic timing tests.
|
||||
|
||||
**Usage**:
|
||||
```typescript
|
||||
const time = createTimeController();
|
||||
// In test:
|
||||
await time.advanceBy(1000); // Advance 1 second
|
||||
await time.runAllTimers(); // Run all pending
|
||||
// In afterEach:
|
||||
time.useRealTimers();
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Port Allocation
|
||||
|
||||
Integration tests that spawn web servers use unique ports to avoid conflicts.
|
||||
|
||||
| Port | Test File | Notes |
|
||||
|-------|------------------------------|----------------------------------|
|
||||
| 3099 | quick-start.test.ts | Basic startup tests |
|
||||
| 3102 | session.test.ts | Session lifecycle tests |
|
||||
| 3105 | scheduled-runs.test.ts | Scheduled task tests |
|
||||
| 3107 | sse-events.test.ts | Server-Sent Events tests |
|
||||
| 3110 | edge-cases.test.ts | Edge case handling |
|
||||
| 3115 | integration-flows.test.ts | End-to-end flows |
|
||||
| 3120 | session-cleanup.test.ts | Session cleanup tests |
|
||||
| 3125 | ralph-integration.test.ts | Ralph loop integration |
|
||||
| 3127 | *Available* | Next integration test |
|
||||
| 3128 | *Available* | Reserved for respawn integration|
|
||||
| 3129+ | *Available* | Future tests |
|
||||
|
||||
### Port Usage Guidelines
|
||||
|
||||
1. **Unit Tests**: No port needed (MockSession, no real server)
|
||||
2. **Integration Tests**: Pick next available port (3127+)
|
||||
3. **Parallel Safety**: `fileParallelism: false` ensures sequential execution
|
||||
|
||||
---
|
||||
|
||||
## Test Isolation Strategy
|
||||
|
||||
### Per-Test Isolation
|
||||
|
||||
1. **Fresh Instances**: Each test creates new MockSession, MockAiIdleChecker, RespawnController
|
||||
2. **No Shared State**: No global variables between tests
|
||||
3. **Timer Cleanup**: Real timers have short timeouts; fake timers reset between tests
|
||||
|
||||
### Per-File Isolation
|
||||
|
||||
1. **beforeEach**: Create fresh instances
|
||||
2. **afterEach**:
|
||||
- Call `controller.stop()` to clear timers
|
||||
- Reset time controller if using fake timers
|
||||
- Clear mock queues
|
||||
|
||||
### Cross-File Isolation
|
||||
|
||||
1. **Sequential Execution**: `fileParallelism: false` in vitest.config.ts
|
||||
2. **Screen Session Limits**: Max 10 concurrent (enforced by setup.ts)
|
||||
3. **Orphan Cleanup**: `afterAll` cleans up any leaked screens
|
||||
|
||||
---
|
||||
|
||||
## Cleanup Procedures
|
||||
|
||||
### During Test Execution
|
||||
|
||||
```typescript
|
||||
afterEach(() => {
|
||||
controller.stop(); // Clear all timers
|
||||
session.removeAllListeners(); // Remove event handlers
|
||||
mockChecker.reset(); // Clear queued results
|
||||
});
|
||||
```
|
||||
|
||||
### After Test Suite
|
||||
|
||||
The global `test/setup.ts` handles:
|
||||
|
||||
1. **Screen Cleanup**: Kills any orphaned `claudeman-*` screens created during tests
|
||||
2. **Process Cleanup**: Kills any Claude processes spawned by tests
|
||||
3. **Pre-existing Protection**: Never kills screens that existed before tests started
|
||||
|
||||
### Emergency Cleanup
|
||||
|
||||
```typescript
|
||||
import { forceCleanupAllTestResources } from './setup.js';
|
||||
|
||||
// Call if tests fail catastrophically
|
||||
forceCleanupAllTestResources();
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Test Categories
|
||||
|
||||
### 1. Unit Tests (MockSession-based)
|
||||
|
||||
Test the RespawnController state machine without real I/O.
|
||||
|
||||
**File**: `test/respawn-controller.test.ts`
|
||||
|
||||
**What to Test**:
|
||||
- State transitions (watching -> confirming_idle -> ai_checking -> ...)
|
||||
- Timer behavior (completion confirm, no-output fallback)
|
||||
- Pattern detection (completion message, working patterns)
|
||||
- Configuration handling
|
||||
- Event emission
|
||||
|
||||
**Example**:
|
||||
```typescript
|
||||
it('should transition to ai_checking when pre-filter met', async () => {
|
||||
const session = createMockSession();
|
||||
const controller = new RespawnController(session, {
|
||||
...FAST_TEST_CONFIG,
|
||||
aiIdleCheckEnabled: true,
|
||||
});
|
||||
|
||||
controller.start();
|
||||
session.simulateCompletionMessage();
|
||||
await new Promise(r => setTimeout(r, 100));
|
||||
|
||||
expect(controller.state).toBe('ai_checking');
|
||||
});
|
||||
```
|
||||
|
||||
### 2. AI Checker Unit Tests
|
||||
|
||||
Test MockAiIdleChecker and MockAiPlanChecker behavior.
|
||||
|
||||
**File**: `test/ai-idle-checker.test.ts`, `test/ai-plan-checker.test.ts`
|
||||
|
||||
**What to Test**:
|
||||
- Verdict queuing
|
||||
- Cooldown behavior
|
||||
- Error handling
|
||||
- Disabled state
|
||||
|
||||
### 3. State Machine Tests
|
||||
|
||||
Comprehensive state transition testing.
|
||||
|
||||
**File**: `test/respawn-controller.test.ts` (State Machine section)
|
||||
|
||||
**What to Test**:
|
||||
- All state transitions in the diagram
|
||||
- Skip paths (sendClear: false, sendInit: false)
|
||||
- Kickstart path
|
||||
- Interruption handling (working patterns during transitions)
|
||||
|
||||
### 4. Integration Tests (if needed)
|
||||
|
||||
Full stack tests with real server but mocked Claude CLI.
|
||||
|
||||
**Port**: 3128 (reserved)
|
||||
|
||||
**What to Test**:
|
||||
- API endpoints for respawn control
|
||||
- SSE event broadcasting
|
||||
- State persistence
|
||||
|
||||
---
|
||||
|
||||
## Usage Examples
|
||||
|
||||
### Basic Test Setup
|
||||
|
||||
```typescript
|
||||
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
|
||||
import { RespawnController } from '../src/respawn-controller.js';
|
||||
import {
|
||||
createMockSession,
|
||||
MockAiIdleChecker,
|
||||
FAST_TEST_CONFIG,
|
||||
createStateTracker,
|
||||
} from './respawn-test-utils.js';
|
||||
|
||||
describe('RespawnController Example', () => {
|
||||
let session: MockSession;
|
||||
let controller: RespawnController;
|
||||
let stateTracker: ReturnType<typeof createStateTracker>;
|
||||
|
||||
beforeEach(() => {
|
||||
session = createMockSession();
|
||||
controller = new RespawnController(session, FAST_TEST_CONFIG);
|
||||
stateTracker = createStateTracker();
|
||||
controller.on('stateChanged', stateTracker.record);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
controller.stop();
|
||||
});
|
||||
|
||||
it('should start in watching state', () => {
|
||||
controller.start();
|
||||
expect(controller.state).toBe('watching');
|
||||
});
|
||||
});
|
||||
```
|
||||
|
||||
### Testing with Mock AI Checker
|
||||
|
||||
```typescript
|
||||
// Note: Currently the RespawnController creates its own AI checkers internally.
|
||||
// To inject mocks, you would need to modify the controller to accept them,
|
||||
// or test the mock checkers independently and test the controller with
|
||||
// aiIdleCheckEnabled: false.
|
||||
|
||||
describe('RespawnController with AI Check Disabled', () => {
|
||||
it('should fall back to direct idle on completion', async () => {
|
||||
const session = createMockSession();
|
||||
const controller = new RespawnController(session, {
|
||||
...FAST_TEST_CONFIG,
|
||||
aiIdleCheckEnabled: false, // Falls back to timer-based
|
||||
});
|
||||
|
||||
let cycleStarted = false;
|
||||
controller.on('respawnCycleStarted', () => { cycleStarted = true; });
|
||||
|
||||
controller.start();
|
||||
session.simulateCompletionMessage();
|
||||
|
||||
await new Promise(r => setTimeout(r, 200));
|
||||
|
||||
expect(cycleStarted).toBe(true);
|
||||
});
|
||||
});
|
||||
```
|
||||
|
||||
### Testing State Transitions
|
||||
|
||||
```typescript
|
||||
describe('State Transitions', () => {
|
||||
it('should track full respawn cycle', async () => {
|
||||
const session = createMockSession();
|
||||
const controller = new RespawnController(session, {
|
||||
...FAST_TEST_CONFIG,
|
||||
sendClear: true,
|
||||
sendInit: true,
|
||||
});
|
||||
const tracker = createStateTracker();
|
||||
controller.on('stateChanged', tracker.record);
|
||||
|
||||
controller.start();
|
||||
session.simulateCompletionMessage();
|
||||
|
||||
// Wait for full cycle
|
||||
await new Promise(r => setTimeout(r, 500));
|
||||
|
||||
expect(tracker.hasVisited('watching')).toBe(true);
|
||||
expect(tracker.hasVisited('confirming_idle')).toBe(true);
|
||||
expect(tracker.hasVisited('sending_update')).toBe(true);
|
||||
expect(tracker.hasVisited('waiting_update')).toBe(true);
|
||||
});
|
||||
});
|
||||
```
|
||||
|
||||
### Testing Event Emission
|
||||
|
||||
```typescript
|
||||
describe('Event Emission', () => {
|
||||
it('should emit all lifecycle events', async () => {
|
||||
const session = createMockSession();
|
||||
const controller = new RespawnController(session, FAST_TEST_CONFIG);
|
||||
const recorder = createEventRecorder();
|
||||
|
||||
controller.on('stateChanged', recorder.handler('stateChanged'));
|
||||
controller.on('respawnCycleStarted', recorder.handler('respawnCycleStarted'));
|
||||
controller.on('stepSent', recorder.handler('stepSent'));
|
||||
|
||||
controller.start();
|
||||
session.simulateCompletionMessage();
|
||||
|
||||
await new Promise(r => setTimeout(r, 200));
|
||||
|
||||
expect(recorder.hasEvent('respawnCycleStarted')).toBe(true);
|
||||
expect(recorder.hasEvent('stepSent')).toBe(true);
|
||||
});
|
||||
});
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## Future Considerations
|
||||
|
||||
### Dependency Injection for AI Checkers
|
||||
|
||||
To enable true mock injection, consider modifying RespawnController to accept optional AI checker instances in the constructor:
|
||||
|
||||
```typescript
|
||||
constructor(
|
||||
session: Session,
|
||||
config: Partial<RespawnConfig> = {},
|
||||
aiChecker?: AiIdleChecker,
|
||||
planChecker?: AiPlanChecker
|
||||
) {
|
||||
// Use provided checkers or create defaults
|
||||
this.aiChecker = aiChecker || new AiIdleChecker(session.id, { ... });
|
||||
this.planChecker = planChecker || new AiPlanChecker(session.id, { ... });
|
||||
}
|
||||
```
|
||||
|
||||
This would allow tests to inject MockAiIdleChecker/MockAiPlanChecker directly.
|
||||
|
||||
### Real AI Checker Tests
|
||||
|
||||
For testing the real AiIdleChecker and AiPlanChecker (which spawn Claude CLI), create a separate test file with:
|
||||
- Longer timeouts
|
||||
- Skip conditions for CI without Claude CLI
|
||||
- Actual screen session usage
|
||||
|
||||
```typescript
|
||||
describe.skipIf(!process.env.CLAUDE_CLI_AVAILABLE)('Real AI Checkers', () => {
|
||||
// Tests that spawn real Claude CLI
|
||||
});
|
||||
```
|
||||
File diff suppressed because it is too large
Load Diff
Reference in New Issue
Block a user