From 1c2aeb0ac40482b48505af2cb3bf8687c365cead Mon Sep 17 00:00:00 2001 From: Ben Houston Date: Tue, 11 Feb 2025 11:26:00 -0500 Subject: [PATCH 1/3] skip global check when checking for updates. --- src/index.ts | 2 ++ src/test-eslint.ts | 5 ---- src/tools/system/shellMessage.test.ts | 2 +- src/tools/system/shellStart.test.ts | 2 +- .../{mockLogger.ts => mockLogger.test.ts} | 6 ++-- src/utils/versionCheck.test.ts | 28 ++----------------- src/utils/versionCheck.ts | 16 ++++------- 7 files changed, 15 insertions(+), 46 deletions(-) delete mode 100644 src/test-eslint.ts rename src/utils/{mockLogger.ts => mockLogger.test.ts} (78%) diff --git a/src/index.ts b/src/index.ts index dfb1fe9..142650c 100644 --- a/src/index.ts +++ b/src/index.ts @@ -23,7 +23,9 @@ const main = async () => { const updateMessage = await checkForUpdates(); if (updateMessage) { + console.log(); logger.info(updateMessage); + console.log(); } // Error handling diff --git a/src/test-eslint.ts b/src/test-eslint.ts deleted file mode 100644 index e94f84e..0000000 --- a/src/test-eslint.ts +++ /dev/null @@ -1,5 +0,0 @@ -function test() { - console.log("Hello"); -} - -export { test }; diff --git a/src/tools/system/shellMessage.test.ts b/src/tools/system/shellMessage.test.ts index 98d9dca..333964c 100644 --- a/src/tools/system/shellMessage.test.ts +++ b/src/tools/system/shellMessage.test.ts @@ -1,6 +1,6 @@ import { describe, it, expect, beforeEach, afterEach } from "vitest"; import { processStates, shellStartTool } from "./shellStart.js"; -import { MockLogger } from "../../utils/mockLogger.js"; +import { MockLogger } from "../../utils/mockLogger.test.js"; import { shellMessageTool } from "./shellMessage.js"; // Helper function to get instanceId from shellStart result diff --git a/src/tools/system/shellStart.test.ts b/src/tools/system/shellStart.test.ts index a4ac1b9..98832a7 100644 --- a/src/tools/system/shellStart.test.ts +++ b/src/tools/system/shellStart.test.ts @@ -1,6 +1,6 @@ import { describe, it, expect, beforeEach, afterEach } from "vitest"; import { processStates, shellStartTool } from "./shellStart.js"; -import { MockLogger } from "../../utils/mockLogger.js"; +import { MockLogger } from "../../utils/mockLogger.test.js"; describe("shellStartTool", () => { const mockLogger = new MockLogger(); diff --git a/src/utils/mockLogger.ts b/src/utils/mockLogger.test.ts similarity index 78% rename from src/utils/mockLogger.ts rename to src/utils/mockLogger.test.ts index 30cde14..efa31ed 100644 --- a/src/utils/mockLogger.ts +++ b/src/utils/mockLogger.test.ts @@ -1,8 +1,8 @@ -import { Logger } from './logger.js'; +import { Logger } from "./logger.js"; export class MockLogger extends Logger { constructor() { - super({ name: 'mock' }); + super({ name: "mock" }); } debug(..._messages: any[]): void {} @@ -10,4 +10,4 @@ export class MockLogger extends Logger { info(..._messages: any[]): void {} warn(..._messages: any[]): void {} error(..._messages: any[]): void {} -} \ No newline at end of file +} diff --git a/src/utils/versionCheck.test.ts b/src/utils/versionCheck.test.ts index b3f0d45..83b4496 100644 --- a/src/utils/versionCheck.test.ts +++ b/src/utils/versionCheck.test.ts @@ -1,40 +1,16 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; import { - isGlobalPackage, generateUpgradeMessage, fetchLatestVersion, getPackageInfo, checkForUpdates, } from "./versionCheck.js"; -// eslint-disable-next-line max-lines-per-function describe("versionCheck", () => { - describe("isGlobalPackage", () => { - const originalEnv = process.env; - - beforeEach(() => { - process.env = { ...originalEnv }; - }); - - afterEach(() => { - process.env = originalEnv; - }); - - it("returns true when npm_config_global is set", () => { - process.env.npm_config_global = "true"; - expect(isGlobalPackage()).toBe(true); - }); - - it("returns false when npm_config_global is not set", () => { - delete process.env.npm_config_global; - expect(isGlobalPackage()).toBe(false); - }); - }); - describe("generateUpgradeMessage", () => { it("returns null when versions are the same", () => { expect(generateUpgradeMessage("1.0.0", "1.0.0", "test-package")).toBe( - null, + null ); }); @@ -67,7 +43,7 @@ describe("versionCheck", () => { const version = await fetchLatestVersion("test-package"); expect(version).toBe("1.1.0"); expect(mockFetch).toHaveBeenCalledWith( - "https://registry.npmjs.org/test-package/latest", + "https://registry.npmjs.org/test-package/latest" ); }); diff --git a/src/utils/versionCheck.ts b/src/utils/versionCheck.ts index dabf3d6..f79b4f9 100644 --- a/src/utils/versionCheck.ts +++ b/src/utils/versionCheck.ts @@ -1,4 +1,5 @@ import { Logger } from "./logger.js"; +import chalk from "chalk"; import { createRequire } from "module"; import type { PackageJson } from "type-fest"; @@ -30,7 +31,7 @@ export function isGlobalPackage(): boolean { * Fetches the latest version of a package from npm registry */ export async function fetchLatestVersion( - packageName: string, + packageName: string ): Promise { try { const registryUrl = `https://registry.npmjs.org/${packageName}/latest`; @@ -45,7 +46,7 @@ export async function fetchLatestVersion( } catch (error) { logger.warn( "Error fetching latest version:", - error instanceof Error ? error.message : String(error), + error instanceof Error ? error.message : String(error) ); return null; } @@ -57,10 +58,10 @@ export async function fetchLatestVersion( export function generateUpgradeMessage( currentVersion: string, latestVersion: string, - packageName: string, + packageName: string ): string | null { return currentVersion !== latestVersion - ? `Update available: ${currentVersion} → ${latestVersion}\nRun 'npm install -g ${packageName}' to update` + ? chalk.green(` Update available: ${currentVersion} → ${latestVersion}\n Run 'npm install -g ${packageName}' to update`) : null; } @@ -72,11 +73,6 @@ export function generateUpgradeMessage( */ export async function checkForUpdates(): Promise { try { - // Only check for updates if running as global package - if (!isGlobalPackage()) { - return null; - } - const { name: packageName, version: currentVersion } = getPackageInfo(); if (!packageName || !currentVersion) { @@ -96,7 +92,7 @@ export async function checkForUpdates(): Promise { // Log error but don't throw to handle gracefully logger.warn( "Error checking for updates:", - error instanceof Error ? error.message : String(error), + error instanceof Error ? error.message : String(error) ); return null; } From 657de5f487ba5448993d5a12633a99ef23f68307 Mon Sep 17 00:00:00 2001 From: Ben Houston Date: Tue, 11 Feb 2025 12:05:02 -0500 Subject: [PATCH 2/3] simplified logging --- src/commands/$default.ts | 10 +- src/commands/tools.ts | 13 +- src/core/toolAgent.test.ts | 16 +- src/index.ts | 12 +- src/tools/interaction/subAgent.test.ts | 4 +- src/tools/io/readFile.test.ts | 4 +- src/tools/io/updateFile.test.ts | 4 +- src/tools/system/shellExecute.test.ts | 4 +- src/tools/system/shellMessage.test.ts | 34 ++-- src/tools/system/shellMessage.ts | 8 +- src/tools/system/shellStart.test.ts | 20 +-- src/tools/system/shellStart.ts | 15 +- src/utils/logger.test.ts | 34 +++- src/utils/logger.ts | 154 +++++++----------- .../{mockLogger.test.ts => mockLogger.ts} | 0 src/utils/versionCheck.test.ts | 11 +- src/utils/versionCheck.ts | 12 +- 17 files changed, 176 insertions(+), 179 deletions(-) rename src/utils/{mockLogger.test.ts => mockLogger.ts} (100%) diff --git a/src/commands/$default.ts b/src/commands/$default.ts index 4ecd368..eebe900 100644 --- a/src/commands/$default.ts +++ b/src/commands/$default.ts @@ -24,7 +24,7 @@ export const builder = (yargs) => { }; export const handler = async (argv: ArgumentsCamelCase) => { - const logger = new Logger({ name: "Default", color: "white" }); + const logger = new Logger({ name: "Default" }); const require = createRequire(import.meta.url); const packageInfo = require("../../package.json"); @@ -33,7 +33,7 @@ export const handler = async (argv: ArgumentsCamelCase) => { "WARNING: This tool can do anything on your command line that you ask it to.", "It can delete files, install software, and even send data to remote servers.", "It is a powerful tool that should be used with caution.", - "By using this tool, you agree that the authors and contributors are not responsible for any damage that may occur as a result of using this tool." + "By using this tool, you agree that the authors and contributors are not responsible for any damage that may occur as a result of using this tool.", ); try { // Early API key check @@ -50,7 +50,7 @@ export const handler = async (argv: ArgumentsCamelCase) => { prompt = await fs.readFile(argv.file, "utf-8"); } catch (error: any) { logger.error( - `Failed to read prompt file: ${argv.file}, ${error?.message}` + `Failed to read prompt file: ${argv.file}, ${error?.message}`, ); process.exit(1); } @@ -65,7 +65,7 @@ export const handler = async (argv: ArgumentsCamelCase) => { try { logger.info( - "Type your request below or 'help' for usage information. Use Ctrl+C to exit." + "Type your request below or 'help' for usage information. Use Ctrl+C to exit.", ); prompt = await readline.question("\n> "); } finally { @@ -78,7 +78,7 @@ export const handler = async (argv: ArgumentsCamelCase) => { if (!prompt) { logger.error( - "No prompt provided. Either specify a prompt, use --promptFile, or run in --interactive mode." + "No prompt provided. Either specify a prompt, use --promptFile, or run in --interactive mode.", ); process.exit(1); } diff --git a/src/commands/tools.ts b/src/commands/tools.ts index c8c9d3f..fe4f214 100644 --- a/src/commands/tools.ts +++ b/src/commands/tools.ts @@ -21,10 +21,9 @@ function formatSchema(schema: { if (schema.properties) { for (const [paramName, param] of Object.entries(schema.properties)) { const required = schema.required?.includes(paramName) - ? " (required)" + ? "" : " (optional)"; - const description = - (param as any).description || "No description available"; + const description = (param as any).description || ""; output += `${paramName}${required}: ${description}\n`; if ((param as any).type) { @@ -61,8 +60,8 @@ export const handler = async () => { tool.parameters as { properties?: Record; required?: string[]; - } - ) + }, + ), ); // Returns section @@ -73,8 +72,8 @@ export const handler = async () => { tool.returns as { properties?: Record; required?: string[]; - } - ) + }, + ), ); } else { console.log(" Type: any"); diff --git a/src/core/toolAgent.test.ts b/src/core/toolAgent.test.ts index 4b7a767..6fb1bc3 100644 --- a/src/core/toolAgent.test.ts +++ b/src/core/toolAgent.test.ts @@ -2,10 +2,10 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; import { executeToolCall } from "./executeToolCall.js"; import { Tool } from "./types.js"; -import { Logger } from "../utils/logger.js"; import { toolAgent } from "./toolAgent.js"; +import { MockLogger } from "../utils/mockLogger.js"; -const logger = new Logger({ name: "toolAgent", logLevel: "warn" }); +const logger = new MockLogger(); // Mock configuration for testing const testConfig = { @@ -96,7 +96,7 @@ describe("toolAgent", () => { input: { input: "test" }, }, [mockTool], - logger, + logger ); expect(result.includes("Processed: test")).toBeTruthy(); @@ -111,8 +111,8 @@ describe("toolAgent", () => { input: {}, }, [mockTool], - logger, - ), + logger + ) ).rejects.toThrow("No tool with the name 'nonexistentTool' exists."); }); @@ -142,8 +142,8 @@ describe("toolAgent", () => { input: {}, }, [errorTool], - logger, - ), + logger + ) ).rejects.toThrow("Deliberate failure"); }); @@ -153,7 +153,7 @@ describe("toolAgent", () => { "Test prompt", [sequenceCompleteTool], logger, - testConfig, + testConfig ); expect(result.result).toBe("Test complete"); diff --git a/src/index.ts b/src/index.ts index 142650c..2b62497 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,7 +1,7 @@ import * as dotenv from "dotenv"; import yargs from "yargs"; import { hideBin } from "yargs/helpers"; -import { Logger } from "./utils/logger.js"; +import { Logger, LogLevel } from "./utils/logger.js"; import { createRequire } from "module"; import { join } from "path"; import { fileURLToPath } from "url"; @@ -16,10 +16,15 @@ import { getTools } from "./tools/getTools.js"; sourceMapSupport.install(); +const nameToLogIndex = (logLevelName: string) => { + // look up the log level name in the enum to get the value + return LogLevel[logLevelName as keyof typeof LogLevel]; +}; + const main = async () => { dotenv.config(); - const logger = new Logger({ name: "Main", color: "white" }); + const logger = new Logger({ name: "Main" }); const updateMessage = await checkForUpdates(); if (updateMessage) { @@ -62,8 +67,7 @@ const main = async () => { // Set up logger with the specified log level argv.logger = new Logger({ name: packageInfo.name!, - color: "white", - logLevel: argv.log, + logLevel: nameToLogIndex(argv.log), }); argv.tools = await getTools(); }) diff --git a/src/tools/interaction/subAgent.test.ts b/src/tools/interaction/subAgent.test.ts index c0bdd8e..92f6ce6 100644 --- a/src/tools/interaction/subAgent.test.ts +++ b/src/tools/interaction/subAgent.test.ts @@ -1,8 +1,8 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; import { subAgentTool } from "./subAgent.js"; -import { Logger } from "../../utils/logger.js"; +import { MockLogger } from "../../utils/mockLogger.js"; -const logger = new Logger({ name: "subAgent", logLevel: "warn" }); +const logger = new MockLogger(); // Mock Anthropic client response const mockResponse = { diff --git a/src/tools/io/readFile.test.ts b/src/tools/io/readFile.test.ts index 53ee251..066851f 100644 --- a/src/tools/io/readFile.test.ts +++ b/src/tools/io/readFile.test.ts @@ -1,8 +1,8 @@ import { describe, it, expect } from "vitest"; -import { Logger } from "../../utils/logger.js"; import { readFileTool } from "./readFile.js"; +import { MockLogger } from "../../utils/mockLogger.js"; -const logger = new Logger({ name: "readFile", logLevel: "warn" }); +const logger = new MockLogger(); describe("readFile", () => { it("should read a file", async () => { diff --git a/src/tools/io/updateFile.test.ts b/src/tools/io/updateFile.test.ts index 2473583..6d7280f 100644 --- a/src/tools/io/updateFile.test.ts +++ b/src/tools/io/updateFile.test.ts @@ -4,12 +4,12 @@ import { join } from "path"; import { randomUUID } from "crypto"; import { mkdtemp } from "fs/promises"; import { tmpdir } from "os"; -import { Logger } from "../../utils/logger.js"; import { updateFileTool } from "./updateFile.js"; import { readFileTool } from "./readFile.js"; import { shellExecuteTool } from "../system/shellExecute.js"; +import { MockLogger } from "../../utils/mockLogger.js"; -const logger = new Logger({ name: "updateFile", logLevel: "warn" }); +const logger = new MockLogger(); describe("updateFile", () => { let testDir: string; diff --git a/src/tools/system/shellExecute.test.ts b/src/tools/system/shellExecute.test.ts index da3bbca..bff247a 100644 --- a/src/tools/system/shellExecute.test.ts +++ b/src/tools/system/shellExecute.test.ts @@ -1,8 +1,8 @@ import { describe, it, expect } from "vitest"; -import { Logger } from "../../utils/logger.js"; import { shellExecuteTool } from "./shellExecute.js"; +import { MockLogger } from "../../utils/mockLogger.js"; -const logger = new Logger({ name: "shellExecute", logLevel: "warn" }); +const logger = new MockLogger(); describe("shellExecute", () => { it("should execute shell commands", async () => { diff --git a/src/tools/system/shellMessage.test.ts b/src/tools/system/shellMessage.test.ts index 333964c..307a797 100644 --- a/src/tools/system/shellMessage.test.ts +++ b/src/tools/system/shellMessage.test.ts @@ -1,11 +1,13 @@ import { describe, it, expect, beforeEach, afterEach } from "vitest"; import { processStates, shellStartTool } from "./shellStart.js"; -import { MockLogger } from "../../utils/mockLogger.test.js"; +import { MockLogger } from "../../utils/mockLogger.js"; import { shellMessageTool } from "./shellMessage.js"; +const logger = new MockLogger(); + // Helper function to get instanceId from shellStart result const getInstanceId = ( - result: Awaited> + result: Awaited>, ) => { if (result.mode === "async") { return result.instanceId; @@ -15,8 +17,6 @@ const getInstanceId = ( // eslint-disable-next-line max-lines-per-function describe("shellMessageTool", () => { - const mockLogger = new MockLogger(); - let testInstanceId = ""; beforeEach(() => { @@ -38,7 +38,7 @@ describe("shellMessageTool", () => { description: "Test interactive process", timeout: 50, // Force async mode for interactive process }, - { logger: mockLogger } + { logger }, ); testInstanceId = getInstanceId(startResult); @@ -50,7 +50,7 @@ describe("shellMessageTool", () => { stdin: "hello world", description: "Test interaction", }, - { logger: mockLogger } + { logger }, ); expect(result.stdout).toBe("hello world"); @@ -64,7 +64,7 @@ describe("shellMessageTool", () => { instanceId: "nonexistent-id", description: "Test invalid process", }, - { logger: mockLogger } + { logger }, ); expect(result.error).toBeDefined(); @@ -79,7 +79,7 @@ describe("shellMessageTool", () => { description: "Test completion", timeout: 0, // Force async mode }, - { logger: mockLogger } + { logger }, ); const instanceId = getInstanceId(startResult); @@ -92,7 +92,7 @@ describe("shellMessageTool", () => { instanceId, description: "Check completion", }, - { logger: mockLogger } + { logger }, ); expect(result.completed).toBe(true); @@ -108,7 +108,7 @@ describe("shellMessageTool", () => { description: "Test SIGTERM handling", timeout: 0, // Force async mode }, - { logger: mockLogger } + { logger }, ); const instanceId = getInstanceId(startResult); @@ -119,7 +119,7 @@ describe("shellMessageTool", () => { signal: "SIGTERM", description: "Send SIGTERM", }, - { logger: mockLogger } + { logger }, ); expect(result.signaled).toBe(true); @@ -130,7 +130,7 @@ describe("shellMessageTool", () => { instanceId, description: "Check on status", }, - { logger: mockLogger } + { logger }, ); expect(result2.completed).toBe(true); @@ -145,7 +145,7 @@ describe("shellMessageTool", () => { description: "Test signal handling on terminated process", timeout: 0, // Force async mode }, - { logger: mockLogger } + { logger }, ); const instanceId = getInstanceId(startResult); @@ -157,7 +157,7 @@ describe("shellMessageTool", () => { signal: "SIGTERM", description: "Send signal to terminated process", }, - { logger: mockLogger } + { logger }, ); expect(result.signaled).toBe(true); @@ -172,7 +172,7 @@ describe("shellMessageTool", () => { description: "Test signal flag verification", timeout: 0, // Force async mode }, - { logger: mockLogger } + { logger }, ); const instanceId = getInstanceId(startResult); @@ -184,7 +184,7 @@ describe("shellMessageTool", () => { signal: "SIGTERM", description: "Send SIGTERM", }, - { logger: mockLogger } + { logger }, ); await new Promise((resolve) => setTimeout(resolve, 50)); @@ -195,7 +195,7 @@ describe("shellMessageTool", () => { instanceId, description: "Check signal state", }, - { logger: mockLogger } + { logger }, ); expect(checkResult.signaled).toBe(true); diff --git a/src/tools/system/shellMessage.ts b/src/tools/system/shellMessage.ts index 801dfa5..8833e1a 100644 --- a/src/tools/system/shellMessage.ts +++ b/src/tools/system/shellMessage.ts @@ -97,7 +97,7 @@ const returnSchema = z signaled: z.boolean().optional(), }) .describe( - "Process interaction results including stdout, stderr, and completion status" + "Process interaction results including stdout, stderr, and completion status", ); type Parameters = z.infer; @@ -112,10 +112,10 @@ export const shellMessageTool: Tool = { execute: async ( { instanceId, stdin, signal }, - { logger } + { logger }, ): Promise => { logger.verbose( - `Interacting with shell process ${instanceId}${stdin ? " with input" : ""}${signal ? ` with signal ${signal}` : ""}` + `Interacting with shell process ${instanceId}${stdin ? " with input" : ""}${signal ? ` with signal ${signal}` : ""}`, ); try { @@ -197,7 +197,7 @@ export const shellMessageTool: Tool = { logParameters: (input, { logger }) => { logger.info( - `Interacting with process ${input.instanceId}, ${input.description}` + `Interacting with process ${input.instanceId}, ${input.description}`, ); }, logReturns: () => {}, diff --git a/src/tools/system/shellStart.test.ts b/src/tools/system/shellStart.test.ts index 98832a7..2ac2929 100644 --- a/src/tools/system/shellStart.test.ts +++ b/src/tools/system/shellStart.test.ts @@ -1,10 +1,10 @@ import { describe, it, expect, beforeEach, afterEach } from "vitest"; import { processStates, shellStartTool } from "./shellStart.js"; -import { MockLogger } from "../../utils/mockLogger.test.js"; +import { MockLogger } from "../../utils/mockLogger.js"; -describe("shellStartTool", () => { - const mockLogger = new MockLogger(); +const logger = new MockLogger(); +describe("shellStartTool", () => { beforeEach(() => { processStates.clear(); }); @@ -23,7 +23,7 @@ describe("shellStartTool", () => { description: "Test process", timeout: 500, // Generous timeout to ensure sync mode }, - { logger: mockLogger } + { logger }, ); expect(result.mode).toBe("sync"); @@ -41,7 +41,7 @@ describe("shellStartTool", () => { description: "Slow command test", timeout: 50, // Short timeout to force async mode }, - { logger: mockLogger } + { logger }, ); expect(result.mode).toBe("async"); @@ -57,7 +57,7 @@ describe("shellStartTool", () => { command: "nonexistentcommand", description: "Invalid command test", }, - { logger: mockLogger } + { logger }, ); expect(result.mode).toBe("sync"); @@ -75,7 +75,7 @@ describe("shellStartTool", () => { description: "Sync completion test", timeout: 500, }, - { logger: mockLogger } + { logger }, ); // Even sync results should be in processStates @@ -88,7 +88,7 @@ describe("shellStartTool", () => { description: "Async completion test", timeout: 50, }, - { logger: mockLogger } + { logger }, ); if (asyncResult.mode === "async") { @@ -103,7 +103,7 @@ describe("shellStartTool", () => { description: "Pipe test", timeout: 50, // Force async for interactive command }, - { logger: mockLogger } + { logger }, ); expect(result.mode).toBe("async"); @@ -136,7 +136,7 @@ describe("shellStartTool", () => { command: "sleep 1", description: "Default timeout test", }, - { logger: mockLogger } + { logger }, ); expect(result.mode).toBe("async"); diff --git a/src/tools/system/shellStart.ts b/src/tools/system/shellStart.ts index 3f8140f..7685610 100644 --- a/src/tools/system/shellStart.ts +++ b/src/tools/system/shellStart.ts @@ -29,7 +29,7 @@ const parameterSchema = z.object({ timeout: z .number() .optional() - .describe("Timeout in ms before switching to async mode (default: 100ms)") + .describe("Timeout in ms before switching to async mode (default: 100ms)"), }); const returnSchema = z.union([ @@ -41,7 +41,9 @@ const returnSchema = z.union([ exitCode: z.number(), error: z.string().optional(), }) - .describe("Synchronous execution results when command completes within timeout"), + .describe( + "Synchronous execution results when command completes within timeout", + ), z .object({ mode: z.literal("async"), @@ -63,7 +65,10 @@ export const shellStartTool: Tool = { parameters: zodToJsonSchema(parameterSchema), returns: zodToJsonSchema(returnSchema), - execute: async ({ command, timeout = 100 }, { logger }): Promise => { + execute: async ( + { command, timeout = 100 }, + { logger }, + ): Promise => { logger.verbose(`Starting shell command: ${command}`); return new Promise((resolve) => { @@ -117,7 +122,7 @@ export const shellStartTool: Tool = { process.on("exit", (code, signal) => { logger.verbose( - `[${instanceId}] Process exited with code ${code} and signal ${signal}` + `[${instanceId}] Process exited with code ${code} and signal ${signal}`, ); processState.state.completed = true; @@ -167,7 +172,7 @@ export const shellStartTool: Tool = { logParameters: (input, { logger }) => { logger.info( - `Starting "${input.command}", ${input.description} (timeout: ${input.timeout}ms)` + `Starting "${input.command}", ${input.description} (timeout: ${input.timeout}ms)`, ); }, logReturns: (output, { logger }) => { diff --git a/src/utils/logger.test.ts b/src/utils/logger.test.ts index 271e907..c4ffc03 100644 --- a/src/utils/logger.test.ts +++ b/src/utils/logger.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; -import { Logger } from "./logger.js"; +import { Logger, LogLevel } from "./logger.js"; describe("Logger", () => { let consoleSpy: { [key: string]: any }; @@ -20,7 +20,7 @@ describe("Logger", () => { }); describe("Basic logging functionality", () => { - const logger = new Logger({ name: "TestLogger", logLevel: "debug" }); + const logger = new Logger({ name: "TestLogger", logLevel: LogLevel.debug }); const testMessage = "Test message"; it("should log debug messages", () => { @@ -59,15 +59,41 @@ describe("Logger", () => { }); }); + describe("Color modes", () => { + const testMessage = "Test message"; + + it("should default to Tool mode for debug/verbose", () => { + const logger = new Logger({ + name: "TestLogger", + logLevel: LogLevel.debug, + }); + logger.debug(testMessage); + expect(consoleSpy.log).toHaveBeenCalledWith( + expect.stringContaining("[TestLogger]"), // Tool mode always shows prefix + ); + }); + + it("should default to Indent mode for info/warn/error", () => { + const logger = new Logger({ + name: "TestLogger", + logLevel: LogLevel.info, + }); + logger.info(testMessage); + expect(consoleSpy.log).toHaveBeenCalledWith( + expect.not.stringContaining("[TestLogger]"), // Indent mode hides prefix for info + ); + }); + }); + describe("Nested logger functionality", () => { const parentLogger = new Logger({ name: "ParentLogger", - logLevel: "debug", + logLevel: LogLevel.debug, }); const childLogger = new Logger({ name: "ChildLogger", parent: parentLogger, - logLevel: "debug", + logLevel: LogLevel.debug, }); const testMessage = "Nested test message"; diff --git a/src/utils/logger.ts b/src/utils/logger.ts index e2eb820..fbeeb90 100644 --- a/src/utils/logger.ts +++ b/src/utils/logger.ts @@ -1,58 +1,79 @@ import chalk, { ChalkInstance } from "chalk"; -export type LogColor = "white" | "blue" | "cyan" | "green" | "magenta"; +export enum LogLevel { + debug = 0, + verbose = 1, + info = 2, + warn = 3, + error = 4, +} +export type LoggerStyler = { + getColor(level: LogLevel, indentLevel: number): ChalkInstance; + formatPrefix(prefix: string, level: LogLevel): string; + showPrefix(level: LogLevel): boolean; +}; -const randomColor = (): LogColor => { - const colors: LogColor[] = ["white", "blue", "cyan", "green", "magenta"]; - return colors[Math.floor(Math.random() * colors.length)] as LogColor; +export const BasicLoggerStyler = { + getColor: (level: LogLevel, _nesting: number = 0): ChalkInstance => { + switch (level) { + case LogLevel.error: + return chalk.red; + case LogLevel.warn: + return chalk.yellow; + case LogLevel.debug: + case LogLevel.verbose: + return chalk.white.dim; + default: + return chalk.white; + } + }, + formatPrefix: ( + prefix: string, + level: LogLevel, + _nesting: number = 0, + ): string => + level === LogLevel.debug || level === LogLevel.verbose + ? chalk.dim(prefix) + : "", }; -export type LogLevel = "info" | "verbose" | "warn" | "error" | "debug"; +const loggerStyle = BasicLoggerStyler; export type LoggerProps = { name: string; - color?: LogColor; logLevel?: LogLevel; parent?: Logger; }; -const LogLevelIndex = { - error: 4, - warn: 3, - info: 2, - verbose: 1, - debug: 0, -}; - export class Logger { - private readonly offset: string; - private readonly chalkColor; + private readonly prefix: string; private readonly logLevel: LogLevel; - private readonly logLevelIndex: number; + private readonly logLevelIndex: LogLevel; private readonly parent?: Logger; private readonly name: string; + private readonly nesting: number; constructor({ name, - color = randomColor(), parent = undefined, - logLevel = parent?.logLevel ?? "info", + logLevel = parent?.logLevel ?? LogLevel.info, }: LoggerProps) { this.name = name; this.parent = parent; this.logLevel = logLevel; - this.logLevelIndex = LogLevelIndex[this.logLevel]; - this.chalkColor = chalk[color]; + this.logLevelIndex = logLevel; - // Calculate offset based on parent chain + // Calculate indent level and offset based on parent chain + this.nesting = 0; let offsetSpaces = 0; let currentParent = parent; while (currentParent) { offsetSpaces += 2; + this.nesting++; currentParent = currentParent.parent; } - this.offset = " ".repeat(offsetSpaces); + this.prefix = " ".repeat(offsetSpaces); } private toStrings(messages: any[]) { @@ -65,95 +86,42 @@ export class Logger { .join(" "); } - private formatMessages( - prefixChalk: ChalkInstance, - prefix: string, - messagesChalk: ChalkInstance, - messages: any[], - showPrefix: boolean = true, - ): string { + private formatMessages(level: LogLevel, messages: any[]): string { const formatted = this.toStrings(messages); + const messageColor = loggerStyle.getColor(level, this.nesting); + const prefix = loggerStyle.formatPrefix( + `[${this.name}]`, + level, + this.nesting, + ); - // Split into lines and add prefix to each line if showPrefix is true return formatted .split("\n") - .map((line) => - showPrefix - ? `${prefixChalk(prefix)} ${messagesChalk(`${line}`)}` - : `${this.offset}${messagesChalk(`${line}`)}`, - ) + .map((line) => `${this.prefix}${prefix} ${messageColor(line)}`) .join("\n"); } - private prefix(): string { - return this.offset + `[${this.name}]`; - } - debug(...messages: any[]): void { - if (this.logLevelIndex > LogLevelIndex.debug) return; - - console.log( - this.formatMessages( - this.chalkColor.dim, - this.prefix(), - this.chalkColor.dim, - messages, - true, // Always show prefix for debug - ), - ); + if (this.logLevelIndex > LogLevel.debug) return; + console.log(this.formatMessages(LogLevel.debug, messages)); } verbose(...messages: any[]): void { - if (this.logLevelIndex > LogLevelIndex.verbose) return; - - console.log( - this.formatMessages( - this.chalkColor.dim, - this.prefix(), - this.chalkColor.dim, - messages, - true, // Always show prefix for verbose - ), - ); + if (this.logLevelIndex > LogLevel.verbose) return; + console.log(this.formatMessages(LogLevel.verbose, messages)); } info(...messages: any[]): void { - if (this.logLevelIndex > LogLevelIndex.info) return; - - console.log( - this.formatMessages( - this.chalkColor, - this.prefix(), - this.chalkColor, - messages, - false, // Don't show prefix for info - ), - ); + if (this.logLevelIndex > LogLevel.info) return; + console.log(this.formatMessages(LogLevel.info, messages)); } warn(...messages: any[]): void { - if (this.logLevelIndex > LogLevelIndex.warn) return; - - console.warn( - this.formatMessages( - this.chalkColor.dim, - this.prefix(), - chalk.yellow, - messages, - false, // Don't show prefix for warn - ), - ); + if (this.logLevelIndex > LogLevel.warn) return; + console.warn(this.formatMessages(LogLevel.warn, messages)); } error(...messages: any[]): void { - console.error( - this.formatMessages( - this.chalkColor.dim, - this.prefix(), - chalk.red, - messages, - false, // Don't show prefix for error - ), - ); + console.error(this.formatMessages(LogLevel.error, messages)); } } diff --git a/src/utils/mockLogger.test.ts b/src/utils/mockLogger.ts similarity index 100% rename from src/utils/mockLogger.test.ts rename to src/utils/mockLogger.ts diff --git a/src/utils/versionCheck.test.ts b/src/utils/versionCheck.test.ts index 83b4496..41e5679 100644 --- a/src/utils/versionCheck.test.ts +++ b/src/utils/versionCheck.test.ts @@ -10,7 +10,7 @@ describe("versionCheck", () => { describe("generateUpgradeMessage", () => { it("returns null when versions are the same", () => { expect(generateUpgradeMessage("1.0.0", "1.0.0", "test-package")).toBe( - null + null, ); }); @@ -43,7 +43,7 @@ describe("versionCheck", () => { const version = await fetchLatestVersion("test-package"); expect(version).toBe("1.1.0"); expect(mockFetch).toHaveBeenCalledWith( - "https://registry.npmjs.org/test-package/latest" + "https://registry.npmjs.org/test-package/latest", ); }); @@ -84,13 +84,6 @@ describe("versionCheck", () => { vi.clearAllMocks(); }); - it("returns null when not running as global package", async () => { - delete process.env.npm_config_global; - const result = await checkForUpdates(); - expect(result).toBe(null); - expect(mockFetch).not.toHaveBeenCalled(); - }); - it("returns upgrade message when update available", async () => { process.env.npm_config_global = "true"; mockFetch.mockResolvedValueOnce({ diff --git a/src/utils/versionCheck.ts b/src/utils/versionCheck.ts index f79b4f9..1c2d6da 100644 --- a/src/utils/versionCheck.ts +++ b/src/utils/versionCheck.ts @@ -31,7 +31,7 @@ export function isGlobalPackage(): boolean { * Fetches the latest version of a package from npm registry */ export async function fetchLatestVersion( - packageName: string + packageName: string, ): Promise { try { const registryUrl = `https://registry.npmjs.org/${packageName}/latest`; @@ -46,7 +46,7 @@ export async function fetchLatestVersion( } catch (error) { logger.warn( "Error fetching latest version:", - error instanceof Error ? error.message : String(error) + error instanceof Error ? error.message : String(error), ); return null; } @@ -58,10 +58,12 @@ export async function fetchLatestVersion( export function generateUpgradeMessage( currentVersion: string, latestVersion: string, - packageName: string + packageName: string, ): string | null { return currentVersion !== latestVersion - ? chalk.green(` Update available: ${currentVersion} → ${latestVersion}\n Run 'npm install -g ${packageName}' to update`) + ? chalk.green( + ` Update available: ${currentVersion} → ${latestVersion}\n Run 'npm install -g ${packageName}' to update`, + ) : null; } @@ -92,7 +94,7 @@ export async function checkForUpdates(): Promise { // Log error but don't throw to handle gracefully logger.warn( "Error checking for updates:", - error instanceof Error ? error.message : String(error) + error instanceof Error ? error.message : String(error), ); return null; } From 71258f1a165cdec6f6a8d527e8bec1c21f9d4fb6 Mon Sep 17 00:00:00 2001 From: Ben Houston Date: Tue, 11 Feb 2025 12:32:50 -0500 Subject: [PATCH 3/3] remove obsolte tests --- src/utils/logger.test.ts | 49 ++++++++++------------------------------ 1 file changed, 12 insertions(+), 37 deletions(-) diff --git a/src/utils/logger.test.ts b/src/utils/logger.test.ts index c4ffc03..53458a0 100644 --- a/src/utils/logger.test.ts +++ b/src/utils/logger.test.ts @@ -1,3 +1,4 @@ +/* eslint-disable max-lines-per-function */ import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; import { Logger, LogLevel } from "./logger.js"; @@ -26,61 +27,35 @@ describe("Logger", () => { it("should log debug messages", () => { logger.debug(testMessage); expect(consoleSpy.log).toHaveBeenCalledWith( - expect.stringContaining(testMessage), + expect.stringContaining(testMessage) ); }); it("should log verbose messages", () => { logger.verbose(testMessage); expect(consoleSpy.log).toHaveBeenCalledWith( - expect.stringContaining(testMessage), + expect.stringContaining(testMessage) ); }); it("should log info messages", () => { logger.info(testMessage); expect(consoleSpy.log).toHaveBeenCalledWith( - expect.stringContaining(testMessage), + expect.stringContaining(testMessage) ); }); it("should log warning messages", () => { logger.warn(testMessage); expect(consoleSpy.warn).toHaveBeenCalledWith( - expect.stringContaining(testMessage), + expect.stringContaining(testMessage) ); }); it("should log error messages", () => { logger.error(testMessage); expect(consoleSpy.error).toHaveBeenCalledWith( - expect.stringContaining(testMessage), - ); - }); - }); - - describe("Color modes", () => { - const testMessage = "Test message"; - - it("should default to Tool mode for debug/verbose", () => { - const logger = new Logger({ - name: "TestLogger", - logLevel: LogLevel.debug, - }); - logger.debug(testMessage); - expect(consoleSpy.log).toHaveBeenCalledWith( - expect.stringContaining("[TestLogger]"), // Tool mode always shows prefix - ); - }); - - it("should default to Indent mode for info/warn/error", () => { - const logger = new Logger({ - name: "TestLogger", - logLevel: LogLevel.info, - }); - logger.info(testMessage); - expect(consoleSpy.log).toHaveBeenCalledWith( - expect.not.stringContaining("[TestLogger]"), // Indent mode hides prefix for info + expect.stringContaining(testMessage) ); }); }); @@ -100,34 +75,34 @@ describe("Logger", () => { it("should include proper indentation for nested loggers", () => { childLogger.info(testMessage); expect(consoleSpy.log).toHaveBeenCalledWith( - expect.stringContaining(" "), // Two spaces of indentation + expect.stringContaining(" ") // Two spaces of indentation ); }); it("should properly log messages at all levels with nested logger", () => { childLogger.debug(testMessage); expect(consoleSpy.log).toHaveBeenCalledWith( - expect.stringContaining(testMessage), + expect.stringContaining(testMessage) ); childLogger.verbose(testMessage); expect(consoleSpy.log).toHaveBeenCalledWith( - expect.stringContaining(testMessage), + expect.stringContaining(testMessage) ); childLogger.info(testMessage); expect(consoleSpy.log).toHaveBeenCalledWith( - expect.stringContaining(testMessage), + expect.stringContaining(testMessage) ); childLogger.warn(testMessage); expect(consoleSpy.warn).toHaveBeenCalledWith( - expect.stringContaining(testMessage), + expect.stringContaining(testMessage) ); childLogger.error(testMessage); expect(consoleSpy.error).toHaveBeenCalledWith( - expect.stringContaining(testMessage), + expect.stringContaining(testMessage) ); }); });