refactor: replace console.log with createLogger in services

- Update feature-loader.ts to use logger.info instead of console.log
- Update dev-server-service.ts to use createLogger for all logging
- Update terminal-service.ts to use createLogger for all logging
- Update agent-service.ts to use createLogger for all logging
- Fix agent-service.test.ts mock to preserve actual createLogger

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
Kacper
2025-12-23 00:41:59 +01:00
parent 0bb9db5f80
commit a1c5d0cb0d
5 changed files with 55 additions and 44 deletions

View File

@@ -11,11 +11,14 @@ import {
buildPromptWithImages, buildPromptWithImages,
isAbortError, isAbortError,
loadContextFiles, loadContextFiles,
createLogger,
} from '@automaker/utils'; } from '@automaker/utils';
import { ProviderFactory } from '../providers/provider-factory.js'; import { ProviderFactory } from '../providers/provider-factory.js';
import { createChatOptions, validateWorkingDirectory } from '../lib/sdk-options.js'; import { createChatOptions, validateWorkingDirectory } from '../lib/sdk-options.js';
import { PathNotAllowedError, secureFs } from '@automaker/platform'; import { PathNotAllowedError, secureFs } from '@automaker/platform';
const logger = createLogger('AgentService');
interface Message { interface Message {
id: string; id: string;
role: 'user' | 'assistant'; role: 'user' | 'assistant';
@@ -149,7 +152,7 @@ export class AgentService {
filename: imageData.filename, filename: imageData.filename,
}); });
} catch (error) { } catch (error) {
console.error(`[AgentService] Failed to load image ${imagePath}:`, error); logger.error(`Failed to load image ${imagePath}:`, error);
} }
} }
} }
@@ -214,9 +217,7 @@ export class AgentService {
// Get provider for this model // Get provider for this model
const provider = ProviderFactory.getProviderForModel(effectiveModel); const provider = ProviderFactory.getProviderForModel(effectiveModel);
console.log( logger.info(`Using provider "${provider.getName()}" for model "${effectiveModel}"`);
`[AgentService] Using provider "${provider.getName()}" for model "${effectiveModel}"`
);
// Build options for provider // Build options for provider
const options: ExecuteOptions = { const options: ExecuteOptions = {
@@ -253,7 +254,7 @@ export class AgentService {
// Capture SDK session ID from any message and persist it // Capture SDK session ID from any message and persist it
if (msg.session_id && !session.sdkSessionId) { if (msg.session_id && !session.sdkSessionId) {
session.sdkSessionId = msg.session_id; session.sdkSessionId = msg.session_id;
console.log(`[AgentService] Captured SDK session ID: ${msg.session_id}`); logger.info(`Captured SDK session ID: ${msg.session_id}`);
// Persist the SDK session ID to ensure conversation continuity across server restarts // Persist the SDK session ID to ensure conversation continuity across server restarts
await this.updateSession(sessionId, { sdkSessionId: msg.session_id }); await this.updateSession(sessionId, { sdkSessionId: msg.session_id });
} }
@@ -329,7 +330,7 @@ export class AgentService {
return { success: false, aborted: true }; return { success: false, aborted: true };
} }
console.error('[AgentService] Error:', error); logger.error('Error:', error);
session.isRunning = false; session.isRunning = false;
session.abortController = null; session.abortController = null;
@@ -423,7 +424,7 @@ export class AgentService {
await secureFs.writeFile(sessionFile, JSON.stringify(messages, null, 2), 'utf-8'); await secureFs.writeFile(sessionFile, JSON.stringify(messages, null, 2), 'utf-8');
await this.updateSessionTimestamp(sessionId); await this.updateSessionTimestamp(sessionId);
} catch (error) { } catch (error) {
console.error('[AgentService] Failed to save session:', error); logger.error('Failed to save session:', error);
} }
} }

View File

@@ -9,9 +9,12 @@
import { spawn, execSync, type ChildProcess } from 'child_process'; import { spawn, execSync, type ChildProcess } from 'child_process';
import { secureFs } from '@automaker/platform'; import { secureFs } from '@automaker/platform';
import { createLogger } from '@automaker/utils';
import path from 'path'; import path from 'path';
import net from 'net'; import net from 'net';
const logger = createLogger('DevServerService');
export interface DevServerInfo { export interface DevServerInfo {
worktreePath: string; worktreePath: string;
port: number; port: number;
@@ -69,7 +72,7 @@ class DevServerService {
for (const pid of pids) { for (const pid of pids) {
try { try {
execSync(`taskkill /F /PID ${pid}`, { stdio: 'ignore' }); execSync(`taskkill /F /PID ${pid}`, { stdio: 'ignore' });
console.log(`[DevServerService] Killed process ${pid} on port ${port}`); logger.info(`Killed process ${pid} on port ${port}`);
} catch { } catch {
// Process may have already exited // Process may have already exited
} }
@@ -82,7 +85,7 @@ class DevServerService {
for (const pid of pids) { for (const pid of pids) {
try { try {
execSync(`kill -9 ${pid}`, { stdio: 'ignore' }); execSync(`kill -9 ${pid}`, { stdio: 'ignore' });
console.log(`[DevServerService] Killed process ${pid} on port ${port}`); logger.info(`Killed process ${pid} on port ${port}`);
} catch { } catch {
// Process may have already exited // Process may have already exited
} }
@@ -93,7 +96,7 @@ class DevServerService {
} }
} catch (error) { } catch (error) {
// Ignore errors - port might not have any process // Ignore errors - port might not have any process
console.log(`[DevServerService] No process to kill on port ${port}`); logger.info(`No process to kill on port ${port}`);
} }
} }
@@ -251,11 +254,9 @@ class DevServerService {
// Small delay to ensure related ports are freed // Small delay to ensure related ports are freed
await new Promise((resolve) => setTimeout(resolve, 100)); await new Promise((resolve) => setTimeout(resolve, 100));
console.log(`[DevServerService] Starting dev server on port ${port}`); logger.info(`Starting dev server on port ${port}`);
console.log(`[DevServerService] Working directory (cwd): ${worktreePath}`); logger.info(`Working directory (cwd): ${worktreePath}`);
console.log( logger.info(`Command: ${devCommand.cmd} ${devCommand.args.join(' ')} with PORT=${port}`);
`[DevServerService] Command: ${devCommand.cmd} ${devCommand.args.join(' ')} with PORT=${port}`
);
// Spawn the dev process with PORT environment variable // Spawn the dev process with PORT environment variable
const env = { const env = {
@@ -276,26 +277,26 @@ class DevServerService {
// Log output for debugging // Log output for debugging
if (devProcess.stdout) { if (devProcess.stdout) {
devProcess.stdout.on('data', (data: Buffer) => { devProcess.stdout.on('data', (data: Buffer) => {
console.log(`[DevServer:${port}] ${data.toString().trim()}`); logger.info(`[DevServer:${port}] ${data.toString().trim()}`);
}); });
} }
if (devProcess.stderr) { if (devProcess.stderr) {
devProcess.stderr.on('data', (data: Buffer) => { devProcess.stderr.on('data', (data: Buffer) => {
const msg = data.toString().trim(); const msg = data.toString().trim();
console.error(`[DevServer:${port}] ${msg}`); logger.error(`[DevServer:${port}] ${msg}`);
}); });
} }
devProcess.on('error', (error) => { devProcess.on('error', (error) => {
console.error(`[DevServerService] Process error:`, error); logger.error(`Process error:`, error);
status.error = error.message; status.error = error.message;
this.allocatedPorts.delete(port); this.allocatedPorts.delete(port);
this.runningServers.delete(worktreePath); this.runningServers.delete(worktreePath);
}); });
devProcess.on('exit', (code) => { devProcess.on('exit', (code) => {
console.log(`[DevServerService] Process for ${worktreePath} exited with code ${code}`); logger.info(`Process for ${worktreePath} exited with code ${code}`);
status.exited = true; status.exited = true;
this.allocatedPorts.delete(port); this.allocatedPorts.delete(port);
this.runningServers.delete(worktreePath); this.runningServers.delete(worktreePath);
@@ -352,9 +353,7 @@ class DevServerService {
// If we don't have a record of this server, it may have crashed/exited on its own // If we don't have a record of this server, it may have crashed/exited on its own
// Return success so the frontend can clear its state // Return success so the frontend can clear its state
if (!server) { if (!server) {
console.log( logger.info(`No server record for ${worktreePath}, may have already stopped`);
`[DevServerService] No server record for ${worktreePath}, may have already stopped`
);
return { return {
success: true, success: true,
result: { result: {
@@ -364,7 +363,7 @@ class DevServerService {
}; };
} }
console.log(`[DevServerService] Stopping dev server for ${worktreePath}`); logger.info(`Stopping dev server for ${worktreePath}`);
// Kill the process // Kill the process
if (server.process && !server.process.killed) { if (server.process && !server.process.killed) {
@@ -434,7 +433,7 @@ class DevServerService {
* Stop all running dev servers (for cleanup) * Stop all running dev servers (for cleanup)
*/ */
async stopAll(): Promise<void> { async stopAll(): Promise<void> {
console.log(`[DevServerService] Stopping all ${this.runningServers.size} dev servers`); logger.info(`Stopping all ${this.runningServers.size} dev servers`);
for (const [worktreePath] of this.runningServers) { for (const [worktreePath] of this.runningServers) {
await this.stopDevServer(worktreePath); await this.stopDevServer(worktreePath);

View File

@@ -57,7 +57,7 @@ export class FeatureLoader {
try { try {
// Paths are now absolute // Paths are now absolute
await secureFs.unlink(oldPath); await secureFs.unlink(oldPath);
console.log(`[FeatureLoader] Deleted orphaned image: ${oldPath}`); logger.info(`Deleted orphaned image: ${oldPath}`);
} catch (error) { } catch (error) {
// Ignore errors when deleting (file may already be gone) // Ignore errors when deleting (file may already be gone)
logger.warn(`[FeatureLoader] Failed to delete image: ${oldPath}`, error); logger.warn(`[FeatureLoader] Failed to delete image: ${oldPath}`, error);
@@ -112,7 +112,7 @@ export class FeatureLoader {
// Copy the file // Copy the file
await secureFs.copyFile(fullOriginalPath, newPath); await secureFs.copyFile(fullOriginalPath, newPath);
console.log(`[FeatureLoader] Copied image: ${originalPath} -> ${newPath}`); logger.info(`Copied image: ${originalPath} -> ${newPath}`);
// Try to delete the original temp file // Try to delete the original temp file
try { try {
@@ -333,7 +333,7 @@ export class FeatureLoader {
try { try {
const featureDir = this.getFeatureDir(projectPath, featureId); const featureDir = this.getFeatureDir(projectPath, featureId);
await secureFs.rm(featureDir, { recursive: true, force: true }); await secureFs.rm(featureDir, { recursive: true, force: true });
console.log(`[FeatureLoader] Deleted feature ${featureId}`); logger.info(`Deleted feature ${featureId}`);
return true; return true;
} catch (error) { } catch (error) {
logger.error(`[FeatureLoader] Failed to delete feature ${featureId}:`, error); logger.error(`[FeatureLoader] Failed to delete feature ${featureId}:`, error);

View File

@@ -10,6 +10,9 @@ import { EventEmitter } from 'events';
import * as os from 'os'; import * as os from 'os';
import * as fs from 'fs'; import * as fs from 'fs';
import * as path from 'path'; import * as path from 'path';
import { createLogger } from '@automaker/utils';
const logger = createLogger('Terminal');
// Maximum scrollback buffer size (characters) // Maximum scrollback buffer size (characters)
const MAX_SCROLLBACK_SIZE = 50000; // ~50KB per terminal const MAX_SCROLLBACK_SIZE = 50000; // ~50KB per terminal
@@ -171,7 +174,7 @@ export class TerminalService extends EventEmitter {
// Reject paths with null bytes (could bypass path checks) // Reject paths with null bytes (could bypass path checks)
if (cwd.includes('\0')) { if (cwd.includes('\0')) {
console.warn(`[Terminal] Rejecting path with null byte: ${cwd.replace(/\0/g, '\\0')}`); logger.warn(`Rejecting path with null byte: ${cwd.replace(/\0/g, '\\0')}`);
return homeDir; return homeDir;
} }
@@ -192,10 +195,10 @@ export class TerminalService extends EventEmitter {
if (stat.isDirectory()) { if (stat.isDirectory()) {
return cwd; return cwd;
} }
console.warn(`[Terminal] Path exists but is not a directory: ${cwd}, falling back to home`); logger.warn(`Path exists but is not a directory: ${cwd}, falling back to home`);
return homeDir; return homeDir;
} catch { } catch {
console.warn(`[Terminal] Working directory does not exist: ${cwd}, falling back to home`); logger.warn(`Working directory does not exist: ${cwd}, falling back to home`);
return homeDir; return homeDir;
} }
} }
@@ -220,7 +223,7 @@ export class TerminalService extends EventEmitter {
setMaxSessions(limit: number): void { setMaxSessions(limit: number): void {
if (limit >= MIN_MAX_SESSIONS && limit <= MAX_MAX_SESSIONS) { if (limit >= MIN_MAX_SESSIONS && limit <= MAX_MAX_SESSIONS) {
maxSessions = limit; maxSessions = limit;
console.log(`[Terminal] Max sessions limit updated to ${limit}`); logger.info(`Max sessions limit updated to ${limit}`);
} }
} }
@@ -231,7 +234,7 @@ export class TerminalService extends EventEmitter {
createSession(options: TerminalOptions = {}): TerminalSession | null { createSession(options: TerminalOptions = {}): TerminalSession | null {
// Check session limit // Check session limit
if (this.sessions.size >= maxSessions) { if (this.sessions.size >= maxSessions) {
console.error(`[Terminal] Max sessions (${maxSessions}) reached, refusing new session`); logger.error(`Max sessions (${maxSessions}) reached, refusing new session`);
return null; return null;
} }
@@ -256,7 +259,7 @@ export class TerminalService extends EventEmitter {
...options.env, ...options.env,
}; };
console.log(`[Terminal] Creating session ${id} with shell: ${shell} in ${cwd}`); logger.info(`Creating session ${id} with shell: ${shell} in ${cwd}`);
const ptyProcess = pty.spawn(shell, shellArgs, { const ptyProcess = pty.spawn(shell, shellArgs, {
name: 'xterm-256color', name: 'xterm-256color',
@@ -328,13 +331,13 @@ export class TerminalService extends EventEmitter {
// Handle exit // Handle exit
ptyProcess.onExit(({ exitCode }) => { ptyProcess.onExit(({ exitCode }) => {
console.log(`[Terminal] Session ${id} exited with code ${exitCode}`); logger.info(`Session ${id} exited with code ${exitCode}`);
this.sessions.delete(id); this.sessions.delete(id);
this.exitCallbacks.forEach((cb) => cb(id, exitCode)); this.exitCallbacks.forEach((cb) => cb(id, exitCode));
this.emit('exit', id, exitCode); this.emit('exit', id, exitCode);
}); });
console.log(`[Terminal] Session ${id} created successfully`); logger.info(`Session ${id} created successfully`);
return session; return session;
} }
@@ -344,7 +347,7 @@ export class TerminalService extends EventEmitter {
write(sessionId: string, data: string): boolean { write(sessionId: string, data: string): boolean {
const session = this.sessions.get(sessionId); const session = this.sessions.get(sessionId);
if (!session) { if (!session) {
console.warn(`[Terminal] Session ${sessionId} not found`); logger.warn(`Session ${sessionId} not found`);
return false; return false;
} }
session.pty.write(data); session.pty.write(data);
@@ -359,7 +362,7 @@ export class TerminalService extends EventEmitter {
resize(sessionId: string, cols: number, rows: number, suppressOutput: boolean = true): boolean { resize(sessionId: string, cols: number, rows: number, suppressOutput: boolean = true): boolean {
const session = this.sessions.get(sessionId); const session = this.sessions.get(sessionId);
if (!session) { if (!session) {
console.warn(`[Terminal] Session ${sessionId} not found for resize`); logger.warn(`Session ${sessionId} not found for resize`);
return false; return false;
} }
try { try {
@@ -385,7 +388,7 @@ export class TerminalService extends EventEmitter {
return true; return true;
} catch (error) { } catch (error) {
console.error(`[Terminal] Error resizing session ${sessionId}:`, error); logger.error(`Error resizing session ${sessionId}:`, error);
session.resizeInProgress = false; // Clear flag on error session.resizeInProgress = false; // Clear flag on error
return false; return false;
} }
@@ -413,14 +416,14 @@ export class TerminalService extends EventEmitter {
} }
// First try graceful SIGTERM to allow process cleanup // First try graceful SIGTERM to allow process cleanup
console.log(`[Terminal] Session ${sessionId} sending SIGTERM`); logger.info(`Session ${sessionId} sending SIGTERM`);
session.pty.kill('SIGTERM'); session.pty.kill('SIGTERM');
// Schedule SIGKILL fallback if process doesn't exit gracefully // Schedule SIGKILL fallback if process doesn't exit gracefully
// The onExit handler will remove session from map when it actually exits // The onExit handler will remove session from map when it actually exits
setTimeout(() => { setTimeout(() => {
if (this.sessions.has(sessionId)) { if (this.sessions.has(sessionId)) {
console.log(`[Terminal] Session ${sessionId} still alive after SIGTERM, sending SIGKILL`); logger.info(`Session ${sessionId} still alive after SIGTERM, sending SIGKILL`);
try { try {
session.pty.kill('SIGKILL'); session.pty.kill('SIGKILL');
} catch { } catch {
@@ -431,10 +434,10 @@ export class TerminalService extends EventEmitter {
} }
}, 1000); }, 1000);
console.log(`[Terminal] Session ${sessionId} kill initiated`); logger.info(`Session ${sessionId} kill initiated`);
return true; return true;
} catch (error) { } catch (error) {
console.error(`[Terminal] Error killing session ${sessionId}:`, error); logger.error(`Error killing session ${sessionId}:`, error);
// Still try to remove from map even if kill fails // Still try to remove from map even if kill fails
this.sessions.delete(sessionId); this.sessions.delete(sessionId);
return false; return false;
@@ -517,7 +520,7 @@ export class TerminalService extends EventEmitter {
* Clean up all sessions * Clean up all sessions
*/ */
cleanup(): void { cleanup(): void {
console.log(`[Terminal] Cleaning up ${this.sessions.size} sessions`); logger.info(`Cleaning up ${this.sessions.size} sessions`);
this.sessions.forEach((session, id) => { this.sessions.forEach((session, id) => {
try { try {
// Clean up flush timeout // Clean up flush timeout

View File

@@ -9,7 +9,15 @@ import { collectAsyncGenerator } from '../../utils/helpers.js';
vi.mock('fs/promises'); vi.mock('fs/promises');
vi.mock('@/providers/provider-factory.js'); vi.mock('@/providers/provider-factory.js');
vi.mock('@automaker/utils'); vi.mock('@automaker/utils', async () => {
const actual = await vi.importActual('@automaker/utils');
return {
...actual,
readImageAsBase64: vi.fn(),
buildPromptWithImages: vi.fn(),
loadContextFiles: vi.fn(),
};
});
describe('agent-service.ts', () => { describe('agent-service.ts', () => {
let service: AgentService; let service: AgentService;