fix(logging): bound log size + honor LOG_LEVEL + mute kicad-skip (#181)
The server wrote gigabytes to ~/.kicad-mcp/logs and ignored LOG_LEVEL. Three root causes, all fixed here (the logging carve-out of #182): - Python (kicad_interface.py): replace the unbounded FileHandler with a RotatingFileHandler (10 MB x 3 backups, env-tunable via KICAD_MCP_LOG_MAX_BYTES / KICAD_MCP_LOG_BACKUP_COUNT); read the level from KICAD_MCP_LOG_LEVEL or LOG_LEVEL (default INFO) instead of hardcoding DEBUG; mute the noisy skip / skip.sexp.* loggers to WARNING unless KICAD_MCP_DEBUG_SKIP is set. - TypeScript (config.ts): honor KICAD_MCP_LOG_LEVEL / LOG_LEVEL for the TS logger. - TypeScript (logger.ts): size-cap the per-day log files with the same env knobs. - Docs + a no-network test for the env helpers, skip muting, and that no unbounded handler targets kicad_interface.log. Verified: LOG_LEVEL is now applied, skip is muted, and the file rotates instead of growing forever. Full suite unchanged from baseline. The hierarchical-sheet rewrite from #182 is intentionally left out (stays as a separate PR pending the #169/#170 design discussion). Co-Authored-By: angelorodem <angelorodem@users.noreply.github.com> Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -16,6 +16,9 @@ const __dirname = dirname(__filename);
|
||||
// Default config location
|
||||
const DEFAULT_CONFIG_PATH = join(dirname(__dirname), "config", "default-config.json");
|
||||
|
||||
const LOG_LEVEL_VALUES = ["error", "warn", "info", "debug"] as const;
|
||||
const LogLevelSchema = z.enum(LOG_LEVEL_VALUES);
|
||||
|
||||
/**
|
||||
* Server configuration schema
|
||||
*/
|
||||
@@ -25,7 +28,7 @@ const ConfigSchema = z.object({
|
||||
description: z.string().default("MCP server for KiCAD PCB design operations"),
|
||||
pythonPath: z.string().optional(),
|
||||
kicadPath: z.string().optional(),
|
||||
logLevel: z.enum(["error", "warn", "info", "debug"]).default("info"),
|
||||
logLevel: LogLevelSchema.default("info"),
|
||||
logDir: z.string().optional(),
|
||||
});
|
||||
|
||||
@@ -34,6 +37,48 @@ const ConfigSchema = z.object({
|
||||
*/
|
||||
export type Config = z.infer<typeof ConfigSchema>;
|
||||
|
||||
/**
|
||||
* Read the log level from the environment (KICAD_MCP_LOG_LEVEL or LOG_LEVEL),
|
||||
* normalizing "warning" -> "warn". Returns undefined when unset/invalid so the
|
||||
* file/default value is kept.
|
||||
*/
|
||||
function getEnvLogLevel(): Config["logLevel"] | undefined {
|
||||
for (const envName of ["KICAD_MCP_LOG_LEVEL", "LOG_LEVEL"] as const) {
|
||||
const rawLevel = process.env[envName];
|
||||
if (!rawLevel) {
|
||||
continue;
|
||||
}
|
||||
|
||||
const normalized = rawLevel.trim().toLowerCase();
|
||||
const level = normalized === "warning" ? "warn" : normalized;
|
||||
const parsed = LogLevelSchema.safeParse(level);
|
||||
if (!parsed.success) {
|
||||
const acceptedValues = [...LOG_LEVEL_VALUES, "warning"].join(", ");
|
||||
logger.warn(
|
||||
`Ignoring invalid ${envName} value: ${rawLevel}. Expected one of: ${acceptedValues}`,
|
||||
);
|
||||
continue;
|
||||
}
|
||||
|
||||
return parsed.data;
|
||||
}
|
||||
|
||||
return undefined;
|
||||
}
|
||||
|
||||
/**
|
||||
* Apply environment-based overrides on top of a loaded config. The env log
|
||||
* level (if set) wins over the file/default so users can control verbosity
|
||||
* without editing the config file.
|
||||
*/
|
||||
function applyEnvironmentOverrides(config: Config): Config {
|
||||
const envLogLevel = getEnvLogLevel();
|
||||
if (!envLogLevel) {
|
||||
return config;
|
||||
}
|
||||
return { ...config, logLevel: envLogLevel };
|
||||
}
|
||||
|
||||
/**
|
||||
* Load configuration from file
|
||||
*
|
||||
@@ -48,7 +93,7 @@ export async function loadConfig(configPath?: string): Promise<Config> {
|
||||
// Check if file exists
|
||||
if (!existsSync(filePath)) {
|
||||
logger.warn(`Configuration file not found: ${filePath}, using defaults`);
|
||||
return ConfigSchema.parse({});
|
||||
return applyEnvironmentOverrides(ConfigSchema.parse({}));
|
||||
}
|
||||
|
||||
// Read and parse configuration
|
||||
@@ -56,11 +101,11 @@ export async function loadConfig(configPath?: string): Promise<Config> {
|
||||
const config = JSON.parse(configData);
|
||||
|
||||
// Validate configuration
|
||||
return ConfigSchema.parse(config);
|
||||
return applyEnvironmentOverrides(ConfigSchema.parse(config));
|
||||
} catch (error) {
|
||||
logger.error(`Error loading configuration: ${error}`);
|
||||
|
||||
// Return default configuration
|
||||
return ConfigSchema.parse({});
|
||||
return applyEnvironmentOverrides(ConfigSchema.parse({}));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -2,7 +2,7 @@
|
||||
* Logger for KiCAD MCP server
|
||||
*/
|
||||
|
||||
import { existsSync, mkdirSync, appendFileSync } from "fs";
|
||||
import { existsSync, mkdirSync, appendFileSync, statSync, renameSync, rmSync } from "fs";
|
||||
import { join } from "path";
|
||||
import * as os from "os";
|
||||
|
||||
@@ -12,12 +12,49 @@ type LogLevel = "error" | "warn" | "info" | "debug";
|
||||
// Default log directory
|
||||
const DEFAULT_LOG_DIR = join(os.homedir(), ".kicad-mcp", "logs");
|
||||
|
||||
/** Parse a non-negative integer env var, or fall back to a default. */
|
||||
function envInt(name: string, fallback: number): number {
|
||||
const raw = process.env[name];
|
||||
if (raw === undefined) return fallback;
|
||||
const value = Number.parseInt(raw, 10);
|
||||
return Number.isFinite(value) && value >= 0 ? value : fallback;
|
||||
}
|
||||
|
||||
/**
|
||||
* Logger class for KiCAD MCP server
|
||||
*/
|
||||
class Logger {
|
||||
private logLevel: LogLevel = "info";
|
||||
private logDir: string = DEFAULT_LOG_DIR;
|
||||
// Size cap for the per-day log files (issue #181). Same env knobs as the
|
||||
// Python side; KICAD_MCP_LOG_MAX_BYTES=0 disables rotation.
|
||||
private maxBytes: number = envInt("KICAD_MCP_LOG_MAX_BYTES", 10 * 1024 * 1024);
|
||||
private backupCount: number = envInt("KICAD_MCP_LOG_BACKUP_COUNT", 3);
|
||||
|
||||
/**
|
||||
* Rotate the day's log file when it exceeds maxBytes so it can't grow
|
||||
* without bound. Best-effort — never throws (logging must not break).
|
||||
*/
|
||||
private rotateIfNeeded(logFile: string): void {
|
||||
if (this.maxBytes <= 0) return;
|
||||
try {
|
||||
if (!existsSync(logFile) || statSync(logFile).size < this.maxBytes) return;
|
||||
if (this.backupCount <= 0) {
|
||||
rmSync(logFile, { force: true });
|
||||
return;
|
||||
}
|
||||
// Drop the oldest, shift .k -> .k+1, then current -> .1
|
||||
const oldest = `${logFile}.${this.backupCount}`;
|
||||
if (existsSync(oldest)) rmSync(oldest, { force: true });
|
||||
for (let i = this.backupCount - 1; i >= 1; i--) {
|
||||
const src = `${logFile}.${i}`;
|
||||
if (existsSync(src)) renameSync(src, `${logFile}.${i + 1}`);
|
||||
}
|
||||
renameSync(logFile, `${logFile}.1`);
|
||||
} catch {
|
||||
// best-effort: if rotation fails, keep logging to the existing file
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Set the log level
|
||||
@@ -103,6 +140,7 @@ class Logger {
|
||||
}
|
||||
|
||||
const logFile = join(this.logDir, `kicad-mcp-${new Date().toISOString().split("T")[0]}.log`);
|
||||
this.rotateIfNeeded(logFile);
|
||||
appendFileSync(logFile, formattedMessage + "\n");
|
||||
} catch (error) {
|
||||
console.error(`Failed to write to log file: ${error}`);
|
||||
|
||||
Reference in New Issue
Block a user