Codeman/docs/code-structure-findings.md

# Code Structure & Quality Findings

**Date**: 2026-02-28
**Scope**: Full codebase analysis across 5 dimensions: frontend, backend, TypeScript, testing, and utilities/config.

This document contains detailed findings for agent teams to write implementation plans and execute improvements. Each section includes severity, specific locations, and recommended fixes.

---

## Table of Contents

1. [Critical: server.ts God Object (6,736 LOC)](#1-critical-serverts-god-object)
2. [Critical: app.js Monolith (15,196 LOC)](#2-critical-appjs-monolith)
3. [Critical: CleanupManager Unused Despite Existing](#3-critical-cleanupmanager-unused)
4. [High: Duplicated Debounce/Timer Patterns](#4-high-duplicated-debouncetimer-patterns)
5. [High: Large Domain Files Need Splitting](#5-high-large-domain-files-need-splitting)
6. [High: types.ts God File (1,443 LOC)](#6-high-typests-god-file)
7. [High: Zod Schemas Duplicate TypeScript Types](#7-high-zod-schemas-duplicate-typescript-types)
8. [High: Test Coverage Gaps](#8-high-test-coverage-gaps)
9. [High: Duplicated Test Mocks](#9-high-duplicated-test-mocks)
10. [Medium: Hardcoded Magic Values](#10-medium-hardcoded-magic-values)
11. [Medium: Frontend Global State Monolith](#11-medium-frontend-global-state-monolith)
12. [Medium: Frontend Code Duplication](#12-medium-frontend-code-duplication)
13. [Medium: Inconsistent Logging](#13-medium-inconsistent-logging)
14. [Medium: Utils Barrel Export Gaps](#14-medium-utils-barrel-export-gaps)
15. [Medium: Non-Null Assertion Risks](#15-medium-non-null-assertion-risks)
16. [Low: Dead Utility Functions](#16-low-dead-utility-functions)
17. [Low: No Dependency Injection for File I/O](#17-low-no-dependency-injection-for-file-io)
18. [Scorecard & Prioritized Roadmap](#18-scorecard--prioritized-roadmap)

---

## 1. Critical: server.ts God Object

**File**: `src/web/server.ts` (6,736 lines)
**Severity**: CRITICAL
**Impact**: Hardest file to maintain, test, and extend. Imports 38 modules.

### Problem

The `WebServer` class handles everything: HTTP routing (~110 routes), authentication, SSE broadcasting, terminal data batching, state persistence, session lifecycle, respawn orchestration, file serving, tunnel management, plan orchestration, and subagent coordination.

**Key metrics**:
- 40+ private properties (Maps, timers, caches)
- 70+ methods
- `setupRoutes()` is 2,000+ LOC of inline route handlers
- Zero test coverage

### Current Structure (Bad)

```
WebServer class (6,736 LOC)
├── Auth session management (lines 469, 668-698)
├── SSE client management (lines 407-408, 5843-5880)
├── Terminal data batching (lines 414-416, 5909-5966)
├── Task update batching (line 426, 5995-6028)
├── State persistence batching (lines 429-430, 6028-6061)
├── Respawn lifecycle (lines 445-451, 5425-5534)
├── Session cleanup (lines 4769-4961)
├── Listener setup (lines 544-643)
└── setupRoutes() (lines 645+, 2000+ LOC)
    ├── /api/sessions/* (30+ routes inline)
    ├── /api/respawn/* (7 routes inline)
    ├── /api/subagents/* (7 routes inline)
    ├── /api/plan/* (5 routes inline)
    ├── /api/push/* (4 routes inline)
    └── ... 60+ more inline
```

### Recommended Structure

```
src/web/
├── server.ts              (~500 LOC - HTTP setup, route registration only)
├── routes/
│   ├── session-routes.ts  (session CRUD, input, resize)
│   ├── respawn-routes.ts  (respawn control endpoints)
│   ├── subagent-routes.ts (background agent tracking)
│   ├── plan-routes.ts     (plan generation & management)
│   ├── push-routes.ts     (web push subscriptions)
│   ├── mux-routes.ts      (tmux management)
│   ├── case-routes.ts     (case management)
│   ├── file-routes.ts     (file browsing/serving)
│   └── system-routes.ts   (status, stats, config, settings)
├── middleware/
│   ├── auth.ts            (Basic Auth + session cookies)
│   └── error-handler.ts   (centralized error responses)
└── services/
    ├── sse-manager.ts     (SSE client + broadcast)
    ├── terminal-batcher.ts (60fps terminal batching)
    └── session-lifecycle.ts (listener setup/teardown)
```

### Duplication in server.ts

**Error response pattern** repeated 189 times:
```typescript
return createErrorResponse(ApiErrorCode.NOT_FOUND, 'Session not found');
```

**Fix**: Extract `findSessionOrFail()` middleware:
```typescript
const findSessionOrFail = (sessionId: string) => {
  const session = this.sessions.get(sessionId);
  if (!session) throw new NotFoundError('Session not found');
  return session;
};
```

**Event listener setup** copy-pasted for subagent watcher, image watcher, and team watcher (lines 544-643). Same attach/detach pattern duplicated 3 times.

---

## 2. Critical: app.js Monolith

**File**: `src/web/public/app.js` (15,196 lines)
**Severity**: CRITICAL
**Impact**: Untestable, hard to navigate, tightly coupled systems.

### Extractable Modules (by priority)

| Module | Lines | Current Location | Impact |
|--------|-------|------------------|--------|
| Mobile handlers (MobileDetection, KeyboardHandler, SwipeHandler) | ~300 | lines 168-620 | High |
| Voice input (DeepgramProvider, VoiceInput) | ~830 | lines 631-1471 | High |
| NotificationManager | ~450 | lines 2218-2663 | High |
| xterm-zerolag-input (inlined copy from packages/) | ~400 | lines 1756-2153 | High |
| KeyboardAccessoryBar | ~195 | lines 1480-1680 | Medium |
| FocusTrap | ~60 | lines 1690-1748 | Medium |

### CodemanApp Class (12,000+ LOC)

The main `CodemanApp` class starting at line 2665 has:
- **60+ Maps/Sets** in the constructor (lines 2667-2805)
- **18 Map instances** with complex cross-references (subagents, parents, teams, windows)
- **10+ monolithic methods** exceeding 100 lines each

**Largest methods**:
| Method | Lines | Size |
|--------|-------|------|
| `renderAppSettings()` | 14400-14700 | ~300 LOC |
| `selectSession()` | 6028-6250 | ~220 LOC |
| `batchTerminalWrite()` | 7482-7700 | ~200 LOC |
| `renderSessionTabs()` | 5814-6000 | ~180 LOC |
| `openSubagentWindow()` | 11927-12100 | ~170 LOC |
| `handleInit()` | 5183-5350 | ~170 LOC |

### Recommended Split

```
src/web/public/
├── app.js                 (~4000 LOC - core app, session mgmt, SSE)
├── mobile.js              (~300 LOC - MobileDetection, KeyboardHandler, SwipeHandler)
├── voice.js               (~830 LOC - DeepgramProvider, VoiceInput)
├── notifications.js       (~450 LOC - NotificationManager)
├── keyboard-accessory.js  (~200 LOC - KeyboardAccessoryBar)
├── api-client.js          (~100 LOC - fetch wrapper with error handling)
└── config.js              (~50 LOC - magic numbers, z-index layers)
```

---

## 3. Critical: CleanupManager Unused

**File**: `src/utils/cleanup-manager.ts` (320 lines)
**Severity**: CRITICAL
**Impact**: Memory leak risk. Well-designed utility exists but is never used. Every file manages cleanup manually.

### Current State

`CleanupManager` is exported from the utils barrel but has **0 instantiations** in production code. Instead, every file implements manual cleanup:

**respawn-controller.ts** (worst offender):
```typescript
// 11 timer properties, manually cleared in stop()
private stepTimer: NodeJS.Timeout | null = null;
private completionConfirmTimer: NodeJS.Timeout | null = null;
private noOutputTimer: NodeJS.Timeout | null = null;
// ... 8 more

stop() {
  if (this.stepTimer) clearTimeout(this.stepTimer);
  if (this.completionConfirmTimer) clearTimeout(this.completionConfirmTimer);
  // ... 9 more clearTimeout/clearInterval calls
}
```

**Files that should use CleanupManager**:
| File | Timer/Listener Count | Current Cleanup |
|------|---------------------|-----------------|
| `respawn-controller.ts` | 11 timers + intervals | 11 manual clearTimeout/clearInterval |
| `web/server.ts` | 6+ timers, debounce map | Manual in stop(), some may leak |
| `state-store.ts` | 2 debounce timers | Manual clearTimeout |
| `push-store.ts` | 1 save timer | Manual clearTimeout |
| `subagent-watcher.ts` | debounce map + watchers | Manual clear + close |
| `ralph-tracker.ts` | 3 debounce timers | Manual clear |
| `bash-tool-parser.ts` | 1 debounce timer | Manual clear |
| `image-watcher.ts` | 1 debounce map | Manual clear |

### Fix

Migrate all timer management to use `CleanupManager`. Example for respawn-controller.ts:

```typescript
// Before: 11 fields + 11 clearTimeout calls
private stepTimer: NodeJS.Timeout | null = null;
// ...

// After: 1 field, auto-cleanup
private cleanup = new CleanupManager();

startStep() {
  this.cleanup.setTimeout(() => { ... }, 5000, 'step');
}

stop() {
  this.cleanup.dispose(); // Clears everything
}
```

---

## 4. High: Duplicated Debounce/Timer Patterns

**Severity**: HIGH
**Impact**: 8+ files implement debounce independently. Bug fixes need to be applied everywhere.

### Pattern Inventory

```typescript
// Pattern 1: Manual timer ref (used in 6 files)
private saveTimer: NodeJS.Timeout | null = null;
debouncedSave() {
  if (this.saveTimer) clearTimeout(this.saveTimer);
  this.saveTimer = setTimeout(() => this.save(), 500);
}

// Pattern 2: Timer Map (used in 3 files)
private fileDebouncers = new Map<string, NodeJS.Timeout>();
debounce(key: string) {
  const existing = this.fileDebouncers.get(key);
  if (existing) clearTimeout(existing);
  this.fileDebouncers.set(key, setTimeout(() => { ... }, 100));
}

// Pattern 3: State flag (used in 2 files)
private isSaving = false;
```

### Locations

| File | Debounce Vars | Delay (ms) |
|------|---------------|------------|
| `state-store.ts` | `saveTimeout`, `ralphStateSaveTimeout` | 500 |
| `push-store.ts` | `saveTimer` | 500 |
| `web/server.ts` | `persistDebounceTimers` (Map) | 500 |
| `subagent-watcher.ts` | `fileDebouncers` (Map) | 100 |
| `ralph-tracker.ts` | 3 debounce timers | 50, 30000 |
| `bash-tool-parser.ts` | `EVENT_DEBOUNCE_MS` | 50 |
| `image-watcher.ts` | debounce map | 200 |
| `respawn-controller.ts` | 11 timer fields | various |

### Fix

Create a `Debouncer` utility:

```typescript
// src/utils/debouncer.ts
export class Debouncer {
  private timer: NodeJS.Timeout | null = null;

  constructor(private readonly delayMs: number) {}

  run(fn: () => void): void {
    if (this.timer) clearTimeout(this.timer);
    this.timer = setTimeout(fn, this.delayMs);
  }

  cancel(): void {
    if (this.timer) clearTimeout(this.timer);
    this.timer = null;
  }
}

// Usage:
private saveDeb = new Debouncer(500);
this.saveDeb.run(() => this.save());
// cleanup: this.saveDeb.cancel();
```

---

## 5. High: Large Domain Files Need Splitting

**Severity**: HIGH
**Impact**: Complex state machines spanning 3,000+ lines are hard to understand and test.

### ralph-tracker.ts (3,905 LOC)

**5 responsibilities mixed**:
1. Output Parsing (~900 LOC) - Line-by-line parsing, state extraction
2. Todo Management (~700 LOC) - Parsing, dedup, expiry
3. Plan Tracking (~800 LOC) - Enhanced plan tasks, checkpoints
4. Circuit Breaker (~400 LOC) - State machine for stuck detection
5. File Watching (~300 LOC) - Monitor external state files

**Recommended split**:
```
ralph-tracker.ts          (core output parsing, ~1200 LOC)
ralph-todo-manager.ts     (todo parsing + management, ~700 LOC)
ralph-plan-tracker.ts     (plan tasks + checkpoints, ~800 LOC)
ralph-circuit-breaker.ts  (circuit breaker logic, ~400 LOC)
```

### respawn-controller.ts (3,611 LOC)

**6 responsibilities mixed**:
1. State Machine (~1,000 LOC) - 6+ states, transitions
2. Idle Detection (~800 LOC) - 5 layers + multi-signal combining
3. AI Checkers (~600 LOC) - Idle + plan checkers integration
4. Health Scoring (~500 LOC) - Metrics, circuit breaker, scoring
5. Action Logging (~300 LOC) - Timeline, detection status
6. Stuck-State Detection (~250 LOC) - Timeout tracking

**Recommended split**:
```
respawn-controller.ts         (state machine core, ~1000 LOC)
respawn-idle-detection.ts     (all 5 idle detection layers, ~800 LOC)
respawn-health-scorer.ts      (metrics & health scoring, ~500 LOC)
```

### session.ts (2,418 LOC)

**8 responsibilities mixed**:
1. PTY Management (~600 LOC)
2. Terminal I/O (~400 LOC)
3. Token Tracking (~200 LOC)
4. Task Tracking (~250 LOC)
5. Ralph Integration (~200 LOC)
6. Auto-Clear/Compact (~300 LOC)
7. Image Watching (~100 LOC)
8. CLI Detection (~150 LOC)

**Recommended split**:
```
session.ts               (PTY + terminal I/O core, ~1000 LOC)
session-tracking.ts      (token + task + Ralph, ~500 LOC)
session-auto-ops.ts      (auto-clear/compact + image, ~300 LOC)
```

---

## 6. High: types.ts God File

**File**: `src/types.ts` (1,443 lines, 72 exported definitions)
**Severity**: HIGH
**Impact**: Every file imports from types.ts. Hard to find relevant types.

### Current Contents

- 46 interfaces
- 25 types
- 1 enum (ApiErrorCode)
- 9 factory functions (createInitialState, etc.)

### Recommended Split

```
src/types/
├── index.ts          (barrel export - transparent migration)
├── session.ts        (SessionState, SessionConfig, SessionMode, SessionColor)
├── task.ts           (TaskState, TaskDefinition, TaskStatus)
├── respawn.ts        (RespawnConfig, RespawnState, CircuitBreakerStatus)
├── ralph.ts          (RalphLoopState, RalphTrackerState, RalphTodoItem)
├── api.ts            (ApiResponse, ApiErrorCode, HookEventType, all route types)
├── lifecycle.ts      (LifecycleEventType, LifecycleEntry)
└── common.ts         (Disposable, BufferConfig, CleanupResourceType)
```

The barrel export makes this a transparent refactor - existing `import from './types'` continues to work.

---

## 7. High: Zod Schemas Duplicate TypeScript Types

**File**: `src/web/schemas.ts` (508 lines)
**Severity**: HIGH
**Impact**: When a type changes, the Zod schema must be manually updated too. Source of bugs.

### Problem

Zod schemas manually duplicate TypeScript interfaces. **Zero `z.infer` usage found.**

```typescript
// types.ts (manual interface)
export interface CreateSessionRequest {
  workingDir?: string;
  mode?: SessionMode;
  name?: string;
}

// schemas.ts (manual Zod schema - duplicated!)
export const CreateSessionSchema = z.object({
  workingDir: safePathSchema.optional(),
  mode: z.enum(['claude', 'shell', 'opencode']).optional(),
  name: z.string().max(100).optional(),
});
```

### Fix

Use `z.infer` to derive TypeScript types from Zod schemas (single source of truth):

```typescript
// schemas.ts
export const CreateSessionSchema = z.object({
  workingDir: safePathSchema.optional(),
  mode: z.enum(['claude', 'shell', 'opencode']).optional(),
  name: z.string().max(100).optional(),
});

// types.ts (auto-derived)
export type CreateSessionRequest = z.infer<typeof CreateSessionSchema>;
```

**Affected schemas** (~10):
- CreateSessionSchema
- RunPromptSchema
- ResizeSchema
- CreateCaseSchema
- QuickStartSchema
- HookEventSchema
- RespawnConfigSchema
- ConfigUpdateSchema
- SettingsUpdateSchema

---

## 8. High: Test Coverage Gaps

**Severity**: HIGH
**Impact**: Critical code paths untested. Regressions go unnoticed.

### Untested Source Files

| File | Lines | Risk |
|------|-------|------|
| `src/web/server.ts` | 6,736 | CRITICAL - Core REST API, 280+ routes |
| `src/plan-orchestrator.ts` | ~500 | HIGH - Multi-agent plan generation |
| `src/tunnel-manager.ts` | ~200 | MEDIUM - Cloudflare tunnel |
| `src/session-lifecycle-log.ts` | ~150 | MEDIUM - JSONL audit log |
| `src/ai-plan-checker.ts` | ~300 | MEDIUM - Plan completion detection |
| `src/templates/claude-md.ts` | ~200 | LOW - CLAUDE.md generation |
| `src/utils/claude-cli-resolver.ts` | ~100 | LOW - CLI path resolution |
| `src/utils/opencode-cli-resolver.ts` | ~100 | LOW - OpenCode CLI support |
| `src/utils/regex-patterns.ts` | ~100 | LOW - Used everywhere! |
| `src/utils/token-validation.ts` | ~50 | LOW - Token counting |

### Test Quality Issues

**10 "not.toThrow()" tests without behavior verification**:
```typescript
// BAD: Only checks it doesn't crash
expect(() => tracker.processMessage(null)).not.toThrow();

// GOOD: Also verify defensive behavior
expect(() => tracker.processMessage(null)).not.toThrow();
expect(tracker.getAllTasks().size).toBe(0);
```

Locations:
- `task-tracker.test.ts` - 5 instances
- `image-watcher.test.ts` - 1 instance
- `task-queue.test.ts` - 1 instance
- Others scattered

---

## 9. High: Duplicated Test Mocks

**Severity**: HIGH
**Impact**: Mock changes need updating in 4 places. Inconsistent mock behavior.

### MockSession Defined 4 Times

| File | Usage |
|------|-------|
| `test/respawn-controller.test.ts` | Full mock with event emitter |
| `test/session-manager.test.ts` | Simpler mock |
| `test/respawn-team-awareness.test.ts` | Copy of respawn-controller mock |
| `test/respawn-test-utils.ts` | **Comprehensive mock - UNUSED!** |

### MockStateStore Defined 2 Times

| File | Usage |
|------|-------|
| `test/session-manager.test.ts` | Basic mock |
| `test/ralph-loop.test.ts` | Separate implementation |

### Unused Test Utilities

`test/respawn-test-utils.ts` exports these utilities that **no test file imports**:
- `createTimeController()` - Abstraction over vitest fake timers
- `MockAiIdleChecker` - Fully mocked AI idle checker
- `MockAiPlanChecker` - Fully mocked plan checker
- Factory functions for pre-configured controllers

### Fix

Create `test/mocks/` directory:
```
test/
├── mocks/
│   ├── mock-session.ts      (single MockSession, used everywhere)
│   ├── mock-state-store.ts  (single MockStateStore)
│   └── index.ts             (barrel export)
├── utils/
│   └── time-controller.ts   (from respawn-test-utils.ts)
└── ... test files
```

---

## 10. Medium: Hardcoded Magic Values

**Severity**: MEDIUM
**Impact**: Hard to tune, inconsistent when same value appears in multiple places.

### Already Centralized (Good)

- `src/config/buffer-limits.ts` - All buffer sizes
- `src/config/map-limits.ts` - All collection limits

### NOT Centralized (40+ values scattered)

**In server.ts** (lines 145-194):
```typescript
const TASK_UPDATE_BATCH_INTERVAL = 100;
const STATE_UPDATE_DEBOUNCE_INTERVAL = 500;
const SESSIONS_LIST_CACHE_TTL = 1000;
const SCHEDULED_CLEANUP_INTERVAL = 5 * 60 * 1000;
const SSE_HEALTH_CHECK_INTERVAL = 30 * 1000;
const MAX_TERMINAL_COLS = 500;
const MAX_TERMINAL_ROWS = 200;
const AUTH_SESSION_TTL_MS = 24 * 60 * 60 * 1000;
const MAX_AUTH_SESSIONS = 100;
const AUTH_FAILURE_WINDOW_MS = 15 * 60 * 1000;
const STATS_COLLECTION_INTERVAL_MS = 2000;
const MAX_INPUT_LENGTH = 64 * 1024;
```

**In hooks-config.ts**: `timeout: 10000` hardcoded 6 times.

**In respawn-controller.ts** (lines 538-565): 10 timing constants.

**In utils**: `EXEC_TIMEOUT_MS = 5000` duplicated in both `claude-cli-resolver.ts` and `opencode-cli-resolver.ts`.

**In app.js**:
```javascript
// line 27: 600000 - stuck detection threshold
// line 24: 5000 - default scrollback
// lines 34-35: 128*1024, 256*1024 - chunk sizes
// lines 152-155: 150, 100 - keyboard detection thresholds
// lines 573-575: 80, 300, 100 - swipe detection params
```

### Fix

Create additional config files:
```
src/config/
├── buffer-limits.ts     (existing)
├── map-limits.ts        (existing)
├── server-config.ts     (NEW - web server intervals, auth, caching)
├── timing-config.ts     (NEW - debounce delays, check intervals)
└── terminal-config.ts   (NEW - max cols/rows, batch intervals)
```

---

## 11. Medium: Frontend Global State Monolith

**Severity**: MEDIUM
**Impact**: All state in single CodemanApp class. Tight coupling between unrelated systems.

### 60+ State Variables in CodemanApp Constructor (lines 2667-2805)

```javascript
this.sessions = new Map();             // Session data
this.subagents = new Map();            // Agent tracking
this.subagentActivity = new Map();     // Tool call tracking
this.subagentToolResults = new Map();  // Result caching
this.subagentParentMap = new Map();    // Agent-to-session mapping
this.teams = new Map();                // Team tracking
this.teamTasks = new Map();           // Team task state
this.planSubagents = new Map();        // Plan agent tracking
this.pendingWrites = [];               // Terminal write queue
this.terminalBufferCache = new Map();  // Buffer caching (unbounded!)
this.projectInsights = new Map();      // Bash tool insights
// ... 40+ more
```

### Problems

1. **18 Map instances** with complex cross-references (no garbage collection strategy)
2. **No domain separation**: Session, subagent, notification, UI, and network state mixed
3. **Implicit dependencies**: `selectSession()` requires 5+ Maps to be in consistent state
4. **`terminalBufferCache`** has no max size - can grow unbounded with many sessions

### Recommended Domain Split

```javascript
// Instead of 60+ flat properties:
class SessionState {
  sessions = new Map();
  sessionOrder = [];
  terminalBuffers = new Map();
  tabAlerts = new Map();
}

class SubagentState {
  subagents = new Map();
  activity = new Map();
  parentMap = new Map();
  windows = new Map();
  minimized = new Map();
}

class TeamState {
  teams = new Map();
  tasks = new Map();
  teammates = new Map();
}

class UIState {
  activeSessionId = null;
  draggedTabId = null;
  isLoadingBuffer = false;
}
```

---

## 12. Medium: Frontend Code Duplication

**Severity**: MEDIUM
**Impact**: Repeated patterns increase maintenance burden and inconsistency risk.

### Duplicated Patterns

**API fetch calls** (~50 instances):
```javascript
// Repeated everywhere:
fetch(`/api/sessions/${sessionId}/...`, {
  method: 'POST',
  headers: { 'Content-Type': 'application/json' },
  body: JSON.stringify({...})
}).catch(() => {})
```
**Fix**: Extract `ApiClient` class.

**`innerHTML` usage** (104 instances):
- Mix of template strings, createElement chains, and direct innerHTML
- Some with manual XSS escaping (`text.replace(/</g, '&lt;')`), some without
- No consistent DOM creation pattern

**`typeof app !== 'undefined'` checks** (20+ instances):
- Lines 458, 467, 481, 614, 617, 1549, etc.
- **Fix**: Ensure `app` is always defined as global singleton.

**Element visibility toggling** (212+ occurrences):
```javascript
element.classList.add('active')
element.classList.remove('active')
```
**Fix**: Create `toggleClass(el, className, condition)` utility.

### Event Listener Issues

- **152 `addEventListener` calls** with fragile cleanup
- **Mix of inline (`onclick="app.method()"`) and addEventListener** - hard to track
- **Element cache (`_elemCache`) never invalidated** if DOM elements are recreated (line 2808)
- **Tab drag-and-drop listeners** may not clean up if user switches tabs mid-drag

---

## 13. Medium: Inconsistent Logging

**Severity**: MEDIUM
**Impact**: Hard to debug in production. Can't filter by severity or component.

### Current State

- **345 console calls** across source files
- **No structured logging** - all `console.log/error` directly
- **No log levels** (DEBUG, INFO, WARN, ERROR)

### Inconsistent Prefixes

```typescript
// Some files use brackets:
console.log('[Session] Starting interactive...');
console.log('[RalphLoop] Task assigned...');
console.log('[TunnelManager] Tunnel started');

// Others use no prefix:
console.error('Failed to spawn PTY:', err);
console.log('Server listening on port', port);
```

### Positive: CleanupManager Has Debug Mode

`src/utils/cleanup-manager.ts` has a `debugMode` flag for conditional debug logging - good pattern not replicated elsewhere.

### Fix

Either:
1. Enforce consistent `[ComponentName]` prefixes via lint rule
2. Create lightweight logger abstraction (not a heavy framework)

---

## 14. Medium: Utils Barrel Export Gaps

**File**: `src/utils/index.ts`
**Severity**: MEDIUM
**Impact**: Forces deep imports, unclear public API.

### Missing Exports

These functions are defined but NOT exported from the barrel:
- `createAnsiPatternFull()` and `createAnsiPatternSimple()` (factory functions from `regex-patterns.ts`)
- `SAFE_PATH_PATTERN` (from `regex-patterns.ts`)
- `validateTokenCounts()` and `validateTokensAndCost()` (from `token-validation.ts`)
- `isSimilar()`, `isSimilarByDistance()`, `levenshteinDistance()`, `normalizePhrase()` (from `string-similarity.ts` - though some are dead code, see finding #16)

### Deep Import Anti-Pattern (16 instances)

Some files bypass the barrel unnecessarily:
```typescript
// Could use barrel:
import { BufferAccumulator } from './utils/buffer-accumulator.js';
import { LRUMap } from './utils/lru-map.js';

// Must deep import (not in barrel):
import { SAFE_PATH_PATTERN } from './utils/regex-patterns.js';
```

### Fix

Add missing exports to `src/utils/index.ts` and update import sites.

---

## 15. Medium: Non-Null Assertion Risks

**Severity**: MEDIUM
**Impact**: Runtime crashes if assumptions violated. 37 instances found.

### Distribution

| File | Count | Risk Level |
|------|-------|------------|
| `src/web/server.ts` | 10 | Low (auth flow verified) |
| `src/session.ts` | 6 | **High** (mux/terminal refs) |
| `src/respawn-controller.ts` | 4 | Low (config validated) |
| `src/lru-map.ts` | 3 | Low (checked lookups) |
| `src/subagent-watcher.ts` | 2 | Low (pending tool calls) |
| Others | 12 | Low |

### High-Risk Examples (session.ts)

```typescript
// Line 915 - _mux could be null if startInteractive called during cleanup
`[Session] Starting interactive (with ${this._mux!.backend})`

// Line 954 - _muxSession could be null in race condition
this._muxSession!.muxName
```

### Fix

Add null guards before assertions, or document invariants:
```typescript
// Before:
this._mux!.backend

// After:
if (!this._mux) throw new Error('Invariant: _mux must be initialized before startInteractive');
this._mux.backend
```

### Positive Notes

- **0 instances of `as any`**
- **0 instances of `@ts-ignore` or `@ts-expect-error`**
- TypeScript overall score: 8.5/10

---

## 16. Low: Dead Utility Functions

**File**: `src/utils/string-similarity.ts`
**Severity**: LOW
**Impact**: Code clutter, confusion about what's actually used.

### Unused Functions

These are defined and exported but **never imported anywhere**:
- `isSimilar(a, b, threshold)` - similarity check with threshold
- `isSimilarByDistance(a, b, maxDistance)` - Levenshtein-based check
- `levenshteinDistance(a, b)` - raw edit distance
- `normalizePhrase(phrase)` - phrase normalization

### Actually Used

Only these are imported from the barrel:
- `stringSimilarity()` - used in ralph-tracker.ts
- `fuzzyPhraseMatch()` - used in ralph-tracker.ts
- `todoContentHash()` - used in ralph-tracker.ts

### Fix

Delete unused functions or mark as `@internal` if kept for future use.

---

## 17. Low: No Dependency Injection for File I/O

**Severity**: LOW (practical impact limited at current scale)
**Impact**: Can't mock filesystem for unit tests. 68+ hard-coded filesystem calls.

### Examples

```typescript
// state-store.ts - directly imports and uses fs
import { readFileSync, writeFileSync, existsSync, mkdirSync } from 'node:fs';

// push-store.ts - hard-coded paths
const KEYS_FILE = join(DATA_DIR, 'push-keys.json');
const SUBS_FILE = join(DATA_DIR, 'push-subscriptions.json');

// ai-checker-base.ts - direct execSync
execSync(`tmux kill-session -t "${this.checkMuxName}"`, { timeout: 3000 });
```

### Why This Is Lower Priority

- The codebase uses integration tests (spawning real processes/tmux sessions) rather than unit tests
- Most filesystem operations are in infrastructure code, not business logic
- Adding DI would be a large refactor with limited near-term benefit

---

## 18. Scorecard & Prioritized Roadmap

### Overall Scores

| Category | Score | Notes |
|----------|-------|-------|
| TypeScript Safety | 8.5/10 | 0 `any`, 0 `@ts-ignore`, 37 non-null assertions |
| Error Handling | 8/10 | Consistent helpers, proper type guards, no untyped catches |
| Async/Promise Safety | 9.5/10 | Zero unhandled rejections, all async properly guarded |
| Resource Cleanup | 7/10 | Good patterns but CleanupManager unused |
| Module Organization | 5/10 | 3 god files, types.ts too large |
| Test Coverage | 6/10 | Core logic tested, server.ts untested |
| Config Centralization | 6/10 | Partially done, 40+ scattered values |
| Frontend Architecture | 4/10 | 15K monolith, 60+ Maps, no modularity |
| Code Duplication | 5/10 | Debounce patterns, fetch calls, mocks |

### Implementation Phases

**Phase 1 - Quick Wins (1-2 days)** ✅ COMPLETE
1. ✅ Export missing functions from utils barrel (~30 min) — `createAnsiPatternFull`, `createAnsiPatternSimple`, `SAFE_PATH_PATTERN`, `validateTokenCounts`, `validateTokensAndCost` all now exported from `src/utils/index.ts`
2. ✅ Delete dead utility functions (~15 min) — `isSimilar()` removed from `string-similarity.ts`; `levenshteinDistance()`, `isSimilarByDistance()`, `normalizePhrase()` made private (used internally by `fuzzyPhraseMatch`/`stringSimilarity`)
3. ✅ Consolidate duplicated `EXEC_TIMEOUT_MS` constant (~15 min) — Created `src/config/exec-timeout.ts` as single source of truth; `claude-cli-resolver.ts`, `opencode-cli-resolver.ts`, and `tmux-manager.ts` all import from it
4. ✅ Add `z.infer` to Zod schemas (~2 hours) — `src/web/schemas.ts` now has 36 `z.infer` type exports (lines 512-547) covering all schemas
5. ✅ Fix 10 weak "not.toThrow()" tests (~1 hour) — All `not.toThrow()` calls now have behavior assertions: `task-tracker.test.ts` (6 instances all followed by state checks), `image-watcher.test.ts` (1 instance followed by length check), `session-manager.test.ts` (1 instance followed by count check)

**Phase 2 - CleanupManager & Debounce (2-3 days)** ✅ COMPLETE
1. ✅ Create `Debouncer` utility class (~1 hour) — Created `src/utils/debouncer.ts` with `Debouncer` and `KeyedDebouncer` classes; exported from `src/utils/index.ts`
2. ✅ Migrate 8 files from manual debounce to Debouncer (~3 hours) — `state-store.ts` (2 Debouncers), `push-store.ts` (1 Debouncer), `ralph-tracker.ts` (2 Debouncers), `bash-tool-parser.ts` (1 Debouncer), `image-watcher.ts` (1 KeyedDebouncer), `subagent-watcher.ts` (1 KeyedDebouncer), `server.ts` (1 KeyedDebouncer for persist timers)
3. ✅ Migrate respawn-controller to CleanupManager (~3 hours) — 10 timer fields replaced with `CleanupManager` instance; `clearTimers()` now uses `cleanup.dispose()` with reinitialization for reuse
4. ✅ Migrate server.ts timer cleanup to CleanupManager (~2 hours) — 5 standalone timers migrated to `CleanupManager`; terminal batch timers and pending respawn starts left as manual Maps (complex lifecycle)
5. ✅ Migrate remaining files (~2 hours) — `ralph-tracker.ts` (watcher + stall timer → CleanupManager), `bash-tool-parser.ts` (auto-remove timers → CleanupManager), `subagent-watcher.ts` (intervals + watchers → CleanupManager); ~30 manual timer fields eliminated, ~200+ lines of boilerplate removed

**Phase 3 - server.ts Route Extraction (3-4 days)**
1. Create `src/web/routes/` directory structure
2. Extract session routes (~2 hours)
3. Extract respawn routes (~1 hour)
4. Extract subagent routes (~1 hour)
5. Extract remaining route groups (~4 hours)
6. Create auth middleware (~1 hour)
7. Create error handler middleware (~1 hour)
8. Reduce server.ts to ~1,500 LOC

**Phase 4 - Domain File Splitting (2-3 days)**
1. Split types.ts into types/ directory (~2 hours)
2. Split ralph-tracker.ts into 4 files (~4 hours)
3. Split respawn-controller.ts into 3 files (~4 hours)
4. Split session.ts into 3 files (~3 hours)

**Phase 5 - Frontend Modularization (3-4 days)**
1. Extract mobile handlers to mobile.js (~2 hours)
2. Extract voice input to voice.js (~3 hours)
3. Extract NotificationManager to notifications.js (~2 hours)
4. Extract KeyboardAccessoryBar (~1 hour)
5. Create ApiClient wrapper (~2 hours)
6. Create config.js for magic numbers (~1 hour)
7. Remove inlined xterm-zerolag-input copy (~1 hour)

**Phase 6 - Config Consolidation (1 day)**
1. Create `src/config/server-config.ts` (~1 hour)
2. Create `src/config/timing-config.ts` (~1 hour)
3. Move scattered constants from 10+ files (~3 hours)

**Phase 7 - Test Infrastructure (2-3 days)**
1. Consolidate MockSession into single shared mock (~2 hours)
2. Create `test/mocks/` with barrel export (~1 hour)
3. Import unused respawn-test-utils where needed (~1 hour)
4. Add server.ts route tests (ongoing, ~8+ hours)

---

## Appendix: File Size Inventory

| File | Lines | Category |
|------|-------|----------|
| `src/web/public/app.js` | 15,196 | Frontend |
| `src/web/server.ts` | 6,736 | Backend |
| `src/ralph-tracker.ts` | 3,905 | Domain |
| `src/respawn-controller.ts` | 3,611 | Domain |
| `src/session.ts` | 2,418 | Domain |
| `src/subagent-watcher.ts` | 1,689 | Domain |
| `src/types.ts` | 1,443 | Types |
| `src/state-store.ts` | 910 | Infrastructure |
| `src/ralph-loop.ts` | ~600 | Domain |
| `src/web/schemas.ts` | 508 | Validation |
| All other src/ files | <500 each | Various |

### What's Working Well

These patterns should be **preserved, not refactored**:
- Clean one-way dependency graph (no circular deps)
- EventEmitter-based decoupling between domain models
- Proper `import type` usage (19 files, consistent)
- Utility type adoption (101 instances of Record, Partial, Omit, etc.)
- `assertNever()` for exhaustive switch checking
- `StaleExpirationMap` and `LRUMap` for bounded collections
- State persistence circuit breaker pattern
- TypeScript strict mode with all safety flags enabled