Skip to content

Commit 28537de

Browse files
Ark0Nclaude
andcommitted
refactor: pass 3 — extract helpers, split long functions, deduplicate patterns
Backend: - subagent-watcher: split 176L processEntry() into 5 focused methods; extract _resolveDescription() deduplicating 3 call sites for description acquisition - bash-tool-parser: split 152L processCleanLine() into 4 handlers; extract _createActiveTool() factory and _scheduleAutoRemove() helper - session: extract _setupOrAttachMuxSession() deduplicating ~80L between startInteractive/startShell; extract _handleTerminalOutput() - respawn-controller: split 180L handleTerminalData() into 3 detection layers; data-driven validation loop replacing 9 individual calls - plan-orchestrator: extract _extractJsonFromResponse(), _emitAgentFailure(), _formatResearchSection() helpers - orchestrator-loop: extract _finalizeTask() unifying task completion/failure; _clearTimer() utility for correct clearInterval/clearTimeout dispatch - ralph-status-parser: config-driven FIELD_PARSERS[] replacing 8 near-identical field-matching blocks; split updateCircuitBreaker() into focused handlers - state-store: extract _mergeWithInitialState() and _resetCircuitBreaker() Frontend: - app.js: add _notifySession() helper used by 18 call sites across 5 modules - panels-ui.js: extract _addActivityEntry() replacing 4 duplicate blocks - settings-ui.js: extract _updateTunnelUrlRow() deduplicating 2 blocks - ralph-panel.js, respawn-ui.js: convert to _notifySession() Routes: - route-helpers: add toggleService() helper - system-routes: use toggleService() for watcher toggles; extract collectActiveTokens() - orchestrator-routes: data-driven EVENT_MAP replacing 10 identical listeners Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent ba09184 commit 28537de

16 files changed

Lines changed: 858 additions & 1033 deletions

src/bash-tool-parser.ts

Lines changed: 99 additions & 115 deletions
Original file line numberDiff line numberDiff line change
@@ -470,120 +470,91 @@ export class BashToolParser extends EventEmitter<BashToolParserEvents> {
470470
* Process a single pre-stripped line of terminal output.
471471
*/
472472
private processCleanLine(cleanLine: string): void {
473-
// Check for tool start
473+
if (this._handleToolStart(cleanLine)) return;
474+
if (this._handleToolCompletion(cleanLine)) return;
475+
if (this._handleTextCommand(cleanLine)) return;
476+
this._handleLogFileMention(cleanLine);
477+
}
478+
479+
private _handleToolStart(cleanLine: string): boolean {
474480
const startMatch = cleanLine.match(BASH_TOOL_START_PATTERN);
475-
if (startMatch) {
476-
const command = startMatch[1];
477-
const timeout = startMatch[2]?.trim();
481+
if (!startMatch) return false;
478482

479-
// Check if this is a file-viewing command
480-
if (this.isFileViewerCommand(command)) {
481-
const filePaths = this.extractFilePaths(command);
483+
const command = startMatch[1];
484+
const timeout = startMatch[2]?.trim();
482485

483-
// Skip if any file path is already tracked (cross-pattern dedup)
484-
if (filePaths.some((fp) => this.isFilePathTracked(fp))) {
485-
return;
486-
}
486+
if (!this.isFileViewerCommand(command)) return true;
487487

488-
if (filePaths.length > 0) {
489-
const tool: ActiveBashTool = {
490-
id: uuidv4(),
491-
command,
492-
filePaths,
493-
timeout,
494-
startedAt: Date.now(),
495-
status: 'running',
496-
sessionId: this._sessionId,
497-
};
498-
499-
// Enforce max tools limit
500-
if (this._activeTools.size >= MAX_ACTIVE_TOOLS) {
501-
// Remove oldest tool (O(n) min-scan instead of O(n log n) sort)
502-
let oldestKey: string | undefined;
503-
let oldestTime = Infinity;
504-
for (const [key, entry] of this._activeTools) {
505-
if (entry.startedAt < oldestTime) {
506-
oldestTime = entry.startedAt;
507-
oldestKey = key;
508-
}
509-
}
510-
if (oldestKey) {
511-
this._activeTools.delete(oldestKey);
512-
}
513-
}
488+
const filePaths = this.extractFilePaths(command);
489+
490+
// Skip if any file path is already tracked (cross-pattern dedup)
491+
if (filePaths.some((fp) => this.isFilePathTracked(fp))) return true;
514492

515-
this._activeTools.set(tool.id, tool);
516-
this._lastToolId = tool.id;
493+
if (filePaths.length > 0) {
494+
const tool = this._createActiveTool(command, filePaths, 'running', timeout);
517495

518-
this.emit('toolStart', tool);
519-
this.scheduleUpdate();
496+
// Enforce max tools limit
497+
if (this._activeTools.size >= MAX_ACTIVE_TOOLS) {
498+
// Remove oldest tool (O(n) min-scan instead of O(n log n) sort)
499+
let oldestKey: string | undefined;
500+
let oldestTime = Infinity;
501+
for (const [key, entry] of this._activeTools) {
502+
if (entry.startedAt < oldestTime) {
503+
oldestTime = entry.startedAt;
504+
oldestKey = key;
505+
}
506+
}
507+
if (oldestKey) {
508+
this._activeTools.delete(oldestKey);
520509
}
521510
}
522-
return;
523-
}
524511

525-
// Check for tool completion
526-
if (TOOL_COMPLETION_PATTERN.test(cleanLine) && this._lastToolId) {
527-
const tool = this._activeTools.get(this._lastToolId);
528-
if (tool && tool.status === 'running') {
529-
tool.status = 'completed';
530-
this.emit('toolEnd', tool);
531-
this.scheduleUpdate();
512+
this._activeTools.set(tool.id, tool);
513+
this._lastToolId = tool.id;
532514

533-
// Remove completed tool after a short delay to allow UI to show completion
534-
this.cleanup.setTimeout(
535-
() => {
536-
if (this._destroyed) return;
537-
this._activeTools.delete(tool.id);
538-
this.scheduleUpdate();
539-
},
540-
2000,
541-
{ description: 'auto-remove completed tool' }
542-
);
543-
}
544-
this._lastToolId = null;
545-
return;
515+
this.emit('toolStart', tool);
516+
this.scheduleUpdate();
546517
}
547518

548-
// Fallback: Check for command suggestions in plain text (e.g., "tail -f /tmp/file.log")
549-
const textCmdMatch = cleanLine.match(TEXT_COMMAND_PATTERN);
550-
if (textCmdMatch) {
551-
const filePath = textCmdMatch[2];
552-
553-
// Create a suggestion tool (marked as 'suggestion' status)
554-
const tool: ActiveBashTool = {
555-
id: uuidv4(),
556-
command: cleanLine.trim(),
557-
filePaths: [filePath],
558-
timeout: undefined,
559-
startedAt: Date.now(),
560-
status: 'running', // Shows as clickable
561-
sessionId: this._sessionId,
562-
};
563-
564-
// Don't add if file path already tracked (cross-pattern dedup)
565-
if (this.isFilePathTracked(filePath)) {
566-
return;
567-
}
519+
return true;
520+
}
568521

569-
this._activeTools.set(tool.id, tool);
570-
this.emit('toolStart', tool);
522+
private _handleToolCompletion(cleanLine: string): boolean {
523+
if (!TOOL_COMPLETION_PATTERN.test(cleanLine) || !this._lastToolId) return false;
524+
525+
const tool = this._activeTools.get(this._lastToolId);
526+
if (tool && tool.status === 'running') {
527+
tool.status = 'completed';
528+
this.emit('toolEnd', tool);
571529
this.scheduleUpdate();
572530

573-
// Auto-remove suggestions after 30 seconds
574-
this.cleanup.setTimeout(
575-
() => {
576-
if (this._destroyed) return;
577-
this._activeTools.delete(tool.id);
578-
this.scheduleUpdate();
579-
},
580-
30000,
581-
{ description: 'auto-remove suggestion tool' }
582-
);
583-
return;
531+
this._scheduleAutoRemove(tool.id, 2000, 'auto-remove completed tool');
584532
}
533+
this._lastToolId = null;
534+
return true;
535+
}
536+
537+
private _handleTextCommand(cleanLine: string): boolean {
538+
const textCmdMatch = cleanLine.match(TEXT_COMMAND_PATTERN);
539+
if (!textCmdMatch) return false;
540+
541+
const filePath = textCmdMatch[2];
542+
543+
// Don't add if file path already tracked (cross-pattern dedup)
544+
if (this.isFilePathTracked(filePath)) return true;
545+
546+
const tool = this._createActiveTool(cleanLine.trim(), [filePath], 'running');
585547

586-
// Last fallback: Check for log file paths mentioned anywhere in the line
548+
this._activeTools.set(tool.id, tool);
549+
this.emit('toolStart', tool);
550+
this.scheduleUpdate();
551+
552+
// Auto-remove suggestions after 30 seconds
553+
this._scheduleAutoRemove(tool.id, 30000, 'auto-remove suggestion tool');
554+
return true;
555+
}
556+
557+
private _handleLogFileMention(cleanLine: string): void {
587558
LOG_FILE_MENTION_PATTERN.lastIndex = 0;
588559
let logMatch;
589560
while ((logMatch = LOG_FILE_MENTION_PATTERN.exec(cleanLine)) !== null) {
@@ -595,33 +566,46 @@ export class BashToolParser extends EventEmitter<BashToolParserEvents> {
595566
// Skip if file path already tracked (cross-pattern dedup)
596567
if (this.isFilePathTracked(filePath)) continue;
597568

598-
const tool: ActiveBashTool = {
599-
id: uuidv4(),
600-
command: `View: ${filePath}`,
601-
filePaths: [filePath],
602-
timeout: undefined,
603-
startedAt: Date.now(),
604-
status: 'running',
605-
sessionId: this._sessionId,
606-
};
569+
const tool = this._createActiveTool(`View: ${filePath}`, [filePath], 'running');
607570

608571
this._activeTools.set(tool.id, tool);
609572
this.emit('toolStart', tool);
610573
this.scheduleUpdate();
611574

612575
// Auto-remove after 60 seconds
613-
this.cleanup.setTimeout(
614-
() => {
615-
if (this._destroyed) return;
616-
this._activeTools.delete(tool.id);
617-
this.scheduleUpdate();
618-
},
619-
60000,
620-
{ description: 'auto-remove log file tool' }
621-
);
576+
this._scheduleAutoRemove(tool.id, 60000, 'auto-remove log file tool');
622577
}
623578
}
624579

580+
private _createActiveTool(
581+
command: string,
582+
filePaths: string[],
583+
status: ActiveBashTool['status'],
584+
timeout?: string
585+
): ActiveBashTool {
586+
return {
587+
id: uuidv4(),
588+
command,
589+
filePaths,
590+
timeout,
591+
startedAt: Date.now(),
592+
status,
593+
sessionId: this._sessionId,
594+
};
595+
}
596+
597+
private _scheduleAutoRemove(toolId: string, delayMs: number, description: string): void {
598+
this.cleanup.setTimeout(
599+
() => {
600+
if (this._destroyed) return;
601+
this._activeTools.delete(toolId);
602+
this.scheduleUpdate();
603+
},
604+
delayMs,
605+
{ description }
606+
);
607+
}
608+
625609
/**
626610
* Check if a command is a file-viewing command worth tracking.
627611
*/

src/orchestrator-loop.ts

Lines changed: 31 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -499,27 +499,34 @@ export class OrchestratorLoop extends EventEmitter {
499499
}
500500
}
501501

502-
private handleTaskCompleted(queueTaskId: string): void {
502+
private _finalizeTask(queueTaskId: string, status: 'completed' | 'failed', error?: string): OrchestratorTask | null {
503503
const orchTask = this.findOrchestratorTaskByQueueId(queueTaskId);
504-
if (!orchTask) return;
504+
if (!orchTask) return null;
505505

506-
orchTask.status = 'completed';
507-
orchTask.completedAt = Date.now();
508-
this.stats.totalTasksCompleted++;
506+
orchTask.status = status;
507+
if (status === 'completed') {
508+
orchTask.completedAt = Date.now();
509+
this.stats.totalTasksCompleted++;
510+
} else {
511+
orchTask.error = error ?? null;
512+
this.stats.totalTasksFailed++;
513+
}
509514
this.persist();
510-
this.emit('taskCompleted', orchTask);
515+
return orchTask;
516+
}
517+
518+
private handleTaskCompleted(queueTaskId: string): void {
519+
const orchTask = this._finalizeTask(queueTaskId, 'completed');
520+
if (!orchTask) return;
511521

522+
this.emit('taskCompleted', orchTask);
512523
this.checkPhaseCompletion();
513524
}
514525

515526
private handleTaskFailed(queueTaskId: string, error: string): void {
516-
const orchTask = this.findOrchestratorTaskByQueueId(queueTaskId);
527+
const orchTask = this._finalizeTask(queueTaskId, 'failed', error);
517528
if (!orchTask) return;
518529

519-
orchTask.status = 'failed';
520-
orchTask.error = error;
521-
this.stats.totalTasksFailed++;
522-
this.persist();
523530
this.emit('taskFailed', orchTask, error);
524531

525532
// Check if we should retry the task or fail the phase
@@ -553,21 +560,22 @@ export class OrchestratorLoop extends EventEmitter {
553560
}, this.config.phaseTimeoutMs);
554561
}
555562

556-
private clearPhasePoll(): void {
557-
if (this.phasePollTimer) {
558-
clearInterval(this.phasePollTimer);
559-
this.phasePollTimer = null;
560-
}
561-
if (this.phaseTimeoutTimer) {
562-
clearTimeout(this.phaseTimeoutTimer);
563-
this.phaseTimeoutTimer = null;
564-
}
565-
if (this.postPhaseTimer) {
566-
clearTimeout(this.postPhaseTimer);
567-
this.postPhaseTimer = null;
563+
private _clearTimer(
564+
timerKey: 'phasePollTimer' | 'phaseTimeoutTimer' | 'postPhaseTimer',
565+
clearFn: typeof clearInterval | typeof clearTimeout
566+
): void {
567+
if (this[timerKey]) {
568+
clearFn(this[timerKey]);
569+
this[timerKey] = null;
568570
}
569571
}
570572

573+
private clearPhasePoll(): void {
574+
this._clearTimer('phasePollTimer', clearInterval);
575+
this._clearTimer('phaseTimeoutTimer', clearTimeout);
576+
this._clearTimer('postPhaseTimer', clearTimeout);
577+
}
578+
571579
private pollPhaseStatus(phase: OrchestratorPhase): void {
572580
// Check for queued tasks that need assignment
573581
const pendingTasks = phase.tasks.filter((t) => t.status === 'pending' && !t.queueTaskId);

0 commit comments

Comments
 (0)