Monorepo: extract shared utils to packages/utils#16
Conversation
📝 WalkthroughWalkthroughCreates a new Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
Summary of ChangesHello @genesiscz, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the project's architecture by introducing a dedicated Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request is a great step towards modularizing the codebase by extracting shared utilities into a new @genesis-tools/utils package. The new package is well-structured with modern tooling, clear entry points, and good dependency management. The use of shims for backwards compatibility is also well-executed. I've identified a few areas for improvement, including a potential bug in the logger's colorization, a reliability issue in an object comparison function, and some inconsistencies in the use of synchronous file system APIs. Overall, this is a solid refactoring effort.
| const newValue = newObj[field]; | ||
|
|
||
| // Deep comparison using JSON.stringify for simplicity | ||
| if (JSON.stringify(oldValue) !== JSON.stringify(newValue)) { |
There was a problem hiding this comment.
Using JSON.stringify for object comparison is unreliable because the order of keys in the resulting string is not guaranteed. For example, { a: 1, b: 2 } and { b: 2, a: 1 } are semantically identical but would be considered different. This can lead to false positives in conflict detection. A proper deep equality check should be used instead. Consider implementing or importing a deepEqual utility function for this.
| try { | ||
| // Dynamic import not supported in sync context, | ||
| // so we provide plain output by default | ||
| // Users can enhance with chalk by passing a colorizer | ||
| } catch { | ||
| // chalk not available, use plain output | ||
| } |
There was a problem hiding this comment.
This try/catch block is empty, which means the colors object is never populated with chalk functions. As a result, the logger will never produce colored output, even if chalk is available. To fix this while respecting chalk as an optional peer dependency, you can use require inside the try block.
try {
// eslint-disable-next-line @typescript-eslint/no-var-requires
const chalk = require('chalk');
colors = {
gray: chalk.gray,
yellow: chalk.yellow,
red: chalk.red,
};
} catch {
// chalk not available, use plain output
}|
|
||
| ```typescript | ||
| import { createServerFn } from '@tanstack/react-start' | ||
| import { Storage } from '@app/utils/storage' |
There was a problem hiding this comment.
The implementation plan references the old storage utility path @app/utils/storage. To keep the documentation consistent with the new architecture, this should be updated to point to the new package.
| import { Storage } from '@app/utils/storage' | |
| import { Storage } from '@genesis-tools/utils/storage' |
|
|
||
| No new dependencies required. Using: | ||
| - Existing TanStack Start infrastructure | ||
| - Existing Storage utility from `@app/utils/storage` |
There was a problem hiding this comment.
This reference to the storage utility should also be updated to the new package path to maintain consistency in the plan.
| - Existing Storage utility from `@app/utils/storage` | |
| - Existing Storage utility from `@genesis-tools/utils/storage` |
| ], | ||
| "repository": { | ||
| "type": "git", | ||
| "url": "https://github.com/your-username/genesis-tools.git" |
There was a problem hiding this comment.
The repository.url field contains a placeholder URL. It's good practice to update this to the actual repository URL for package discoverability and correctness on npm.
| export function fileExists(path: string): boolean { | ||
| const { existsSync } = require('node:fs'); | ||
| return existsSync(path); | ||
| } |
There was a problem hiding this comment.
The fileExists function uses the synchronous existsSync. This is inconsistent with the other async file operations in this module. It's better to make it async and use the appropriate runtime-native async APIs for both Bun and Node.js. This would also involve making isCacheFileExpired in storage.ts async.
| export function fileExists(path: string): boolean { | |
| const { existsSync } = require('node:fs'); | |
| return existsSync(path); | |
| } | |
| export async function fileExists(path: string): Promise<boolean> { | |
| if (isBun) { | |
| return Bun!.file(path).exists(); | |
| } | |
| const { promises: fsPromises } = require('node:fs'); | |
| try { | |
| await fsPromises.access(path); | |
| return true; | |
| } catch { | |
| return false; | |
| } | |
| } |
| export class Storage { | ||
| private toolName: string; | ||
| private baseDir: string; | ||
| private cacheDir: string; | ||
| private configPath: string; | ||
| private logger: StorageLogger; | ||
|
|
||
| /** | ||
| * Create a Storage instance for a tool | ||
| * @param toolName - Name of the tool (e.g., "timely", "ask") | ||
| * @param logger - Optional logger instance (defaults to no-op) | ||
| */ | ||
| constructor(toolName: string, logger: StorageLogger = noopLogger) { | ||
| this.toolName = toolName; | ||
| this.baseDir = join(homedir(), ".genesis-tools", toolName); | ||
| this.cacheDir = join(this.baseDir, "cache"); | ||
| this.configPath = join(this.baseDir, "config.json"); | ||
| this.logger = logger; | ||
| } | ||
|
|
||
| // ============================================ | ||
| // Directory Management | ||
| // ============================================ | ||
|
|
||
| /** | ||
| * Get the base directory for this tool | ||
| * @returns Absolute path to ~/.genesis-tools/<toolName> | ||
| */ | ||
| getBaseDir(): string { | ||
| return this.baseDir; | ||
| } | ||
|
|
||
| /** | ||
| * Get the cache directory for this tool | ||
| * @returns Absolute path to ~/.genesis-tools/<toolName>/cache | ||
| */ | ||
| getCacheDir(): string { | ||
| return this.cacheDir; | ||
| } | ||
|
|
||
| /** | ||
| * Get the config file path | ||
| * @returns Absolute path to ~/.genesis-tools/<toolName>/config.json | ||
| */ | ||
| getConfigPath(): string { | ||
| return this.configPath; | ||
| } | ||
|
|
||
| /** | ||
| * Ensure all required directories exist | ||
| * Creates: baseDir, cacheDir | ||
| */ | ||
| async ensureDirs(): Promise<void> { | ||
| if (!existsSync(this.baseDir)) { | ||
| mkdirSync(this.baseDir, { recursive: true }); | ||
| this.logger.debug(`Created directory: ${this.baseDir}`); | ||
| } | ||
|
|
||
| if (!existsSync(this.cacheDir)) { | ||
| mkdirSync(this.cacheDir, { recursive: true }); | ||
| this.logger.debug(`Created directory: ${this.cacheDir}`); | ||
| } | ||
| } | ||
|
|
||
| // ============================================ | ||
| // Config Management | ||
| // ============================================ | ||
|
|
||
| /** | ||
| * Read the entire config object | ||
| * @returns The config object or null if not found | ||
| */ | ||
| async getConfig<T extends object>(): Promise<T | null> { | ||
| try { | ||
| if (!existsSync(this.configPath)) { | ||
| return null; | ||
| } | ||
| const content = await readFile(this.configPath); | ||
| return JSON.parse(content) as T; | ||
| } catch (error) { | ||
| this.logger.error(`Failed to read config: ${error}`); | ||
| return null; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Get a specific value from config | ||
| * @param key - The config key (supports dot notation: "oauth2.access_token") | ||
| * @returns The value or undefined | ||
| */ | ||
| async getConfigValue<T>(key: string): Promise<T | undefined> { | ||
| const config = await this.getConfig<Record<string, unknown>>(); | ||
| if (!config) return undefined; | ||
|
|
||
| // Support dot notation | ||
| const keys = key.split("."); | ||
| let value: unknown = config; | ||
| for (const k of keys) { | ||
| if (value && typeof value === "object" && k in value) { | ||
| value = (value as Record<string, unknown>)[k]; | ||
| } else { | ||
| return undefined; | ||
| } | ||
| } | ||
| return value as T; | ||
| } | ||
|
|
||
| /** | ||
| * Set a value in config (merges with existing config) | ||
| * @param key - The config key (supports dot notation) | ||
| * @param value - The value to set | ||
| */ | ||
| async setConfigValue<T>(key: string, value: T): Promise<void> { | ||
| await this.ensureDirs(); | ||
| const config = (await this.getConfig<Record<string, unknown>>()) || {}; | ||
|
|
||
| // Support dot notation for nested keys | ||
| const keys = key.split("."); | ||
| let current = config; | ||
| for (let i = 0; i < keys.length - 1; i++) { | ||
| const k = keys[i]; | ||
| if (!(k in current) || typeof current[k] !== "object") { | ||
| current[k] = {}; | ||
| } | ||
| current = current[k] as Record<string, unknown>; | ||
| } | ||
| current[keys[keys.length - 1]] = value; | ||
|
|
||
| await writeFile(this.configPath, JSON.stringify(config, null, 2)); | ||
| this.logger.debug(`Config updated: ${key}`); | ||
| } | ||
|
|
||
| /** | ||
| * Set the entire config object | ||
| * @param config - The config object to save | ||
| */ | ||
| async setConfig<T extends object>(config: T): Promise<void> { | ||
| await this.ensureDirs(); | ||
| await writeFile(this.configPath, JSON.stringify(config, null, 2)); | ||
| this.logger.debug(`Config saved`); | ||
| } | ||
|
|
||
| /** | ||
| * Clear the config (delete config.json) | ||
| */ | ||
| async clearConfig(): Promise<void> { | ||
| try { | ||
| if (existsSync(this.configPath)) { | ||
| unlinkSync(this.configPath); | ||
| this.logger.debug(`Config cleared`); | ||
| } | ||
| } catch (error) { | ||
| this.logger.error(`Failed to clear config: ${error}`); | ||
| } | ||
| } | ||
|
|
||
| // ============================================ | ||
| // Cache Management | ||
| // ============================================ | ||
|
|
||
| /** | ||
| * Parse TTL string to milliseconds | ||
| * @param ttl - TTL string like "5 days", "1 hour", "30 minutes" | ||
| * @returns Milliseconds | ||
| */ | ||
| parseTTL(ttl: TTLString): number { | ||
| const match = ttl.trim().match(/^(\d+)\s*(second|minute|hour|day|week)s?$/i); | ||
| if (!match) { | ||
| throw new Error(`Invalid TTL format: "${ttl}". Use format like "5 days", "1 hour", "30 minutes"`); | ||
| } | ||
|
|
||
| const value = parseInt(match[1], 10); | ||
| const unit = match[2].toLowerCase(); | ||
|
|
||
| const multipliers: Record<string, number> = { | ||
| second: 1000, | ||
| minute: 60 * 1000, | ||
| hour: 60 * 60 * 1000, | ||
| day: 24 * 60 * 60 * 1000, | ||
| week: 7 * 24 * 60 * 60 * 1000, | ||
| }; | ||
|
|
||
| return value * multipliers[unit]; | ||
| } | ||
|
|
||
| /** | ||
| * Get the full path for a cache file | ||
| * @param relativePath - Relative path within cache directory | ||
| * @returns Absolute path | ||
| */ | ||
| private getCacheFilePath(relativePath: string): string { | ||
| return join(this.cacheDir, relativePath); | ||
| } | ||
|
|
||
| /** | ||
| * Check if a cache file is expired based on file modification time | ||
| * @param filePath - Absolute path to the cache file | ||
| * @param ttlMs - TTL in milliseconds | ||
| * @returns true if expired or doesn't exist | ||
| */ | ||
| private isCacheFileExpired(filePath: string, ttlMs: number): boolean { | ||
| try { | ||
| if (!existsSync(filePath)) { | ||
| return true; | ||
| } | ||
|
|
||
| const stats = statSync(filePath); | ||
| const age = Date.now() - stats.mtimeMs; | ||
| return age > ttlMs; | ||
| } catch { | ||
| return true; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Put a file in the cache (saves as raw JSON, no metadata wrapper) | ||
| * Expiration is checked via file modification time | ||
| * @param relativePath - Relative path within cache directory | ||
| * @param data - Data to cache (will be JSON stringified) | ||
| * @param _ttl - TTL string (expiration checked via file mtime) | ||
| */ | ||
| async putCacheFile<T>(relativePath: string, data: T, _ttl: TTLString): Promise<void> { | ||
| await this.ensureDirs(); | ||
| const filePath = this.getCacheFilePath(relativePath); | ||
|
|
||
| // Ensure parent directory exists | ||
| const dir = dirname(filePath); | ||
| if (!existsSync(dir)) { | ||
| mkdirSync(dir, { recursive: true }); | ||
| } | ||
|
|
||
| // Save raw JSON without metadata wrapper | ||
| const content = JSON.stringify(data, null, 2); | ||
| await writeFile(filePath, content); | ||
| this.logger.debug(`Cache written: ${filePath}`); | ||
| } | ||
|
|
||
| /** | ||
| * Get a file from cache (returns null if not found or expired) | ||
| * Expiration is checked based on file modification time | ||
| * @param relativePath - Relative path within cache directory | ||
| * @param ttl - TTL string to check expiration | ||
| * @returns Cached data or null | ||
| */ | ||
| async getCacheFile<T>(relativePath: string, ttl: TTLString): Promise<T | null> { | ||
| const filePath = this.getCacheFilePath(relativePath); | ||
| const ttlMs = this.parseTTL(ttl); | ||
|
|
||
| if (this.isCacheFileExpired(filePath, ttlMs)) { | ||
| return null; | ||
| } | ||
|
|
||
| try { | ||
| const content = await readFile(filePath); | ||
| return JSON.parse(content) as T; | ||
| } catch { | ||
| return null; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Get cached file or fetch and cache it | ||
| * @param relativePath - Relative path within cache directory | ||
| * @param fetcher - Async function to fetch data if not cached | ||
| * @param ttl - TTL string like "5 days" | ||
| * @returns Cached or fetched data | ||
| */ | ||
| async getFileOrPut<T>(relativePath: string, fetcher: () => Promise<T>, ttl: TTLString): Promise<T> { | ||
| const filePath = this.getCacheFilePath(relativePath); | ||
|
|
||
| // Try to get from cache first | ||
| const cached = await this.getCacheFile<T>(relativePath, ttl); | ||
| if (cached !== null) { | ||
| this.logger.debug(`Cache hit: ${filePath}`); | ||
| return cached; | ||
| } | ||
|
|
||
| // Fetch fresh data | ||
| this.logger.debug(`Cache miss: ${filePath}, fetching...`); | ||
| const data = await fetcher(); | ||
|
|
||
| // Store in cache | ||
| await this.putCacheFile(relativePath, data, ttl); | ||
|
|
||
| return data; | ||
| } | ||
|
|
||
| /** | ||
| * Delete a specific cache file | ||
| * @param relativePath - Relative path within cache directory | ||
| */ | ||
| async deleteCacheFile(relativePath: string): Promise<void> { | ||
| const filePath = this.getCacheFilePath(relativePath); | ||
| try { | ||
| if (existsSync(filePath)) { | ||
| unlinkSync(filePath); | ||
| this.logger.debug(`Cache deleted: ${relativePath}`); | ||
| } | ||
| } catch (error) { | ||
| this.logger.error(`Failed to delete cache file: ${error}`); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Clear all cache files | ||
| */ | ||
| async clearCache(): Promise<void> { | ||
| try { | ||
| const removeDir = (dir: string) => { | ||
| if (!existsSync(dir)) return; | ||
| const files = readdirSync(dir, { withFileTypes: true }); | ||
| for (const file of files) { | ||
| const filePath = join(dir, file.name); | ||
| if (file.isDirectory()) { | ||
| removeDir(filePath); | ||
| } else { | ||
| unlinkSync(filePath); | ||
| } | ||
| } | ||
| }; | ||
| removeDir(this.cacheDir); | ||
| // Recreate empty cache directory | ||
| mkdirSync(this.cacheDir, { recursive: true }); | ||
| this.logger.debug(`Cache cleared for ${this.toolName}`); | ||
| } catch (error) { | ||
| this.logger.error(`Failed to clear cache: ${error}`); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * List all cache files | ||
| * @param absolute - If true, returns absolute paths; if false, returns relative paths (default: true) | ||
| * @returns Array of paths (absolute or relative based on parameter) | ||
| */ | ||
| async listCacheFiles(absolute: boolean = true): Promise<string[]> { | ||
| const files: string[] = []; | ||
|
|
||
| const walkDir = (dir: string, prefix: string = "") => { | ||
| if (!existsSync(dir)) return; | ||
| const entries = readdirSync(dir, { withFileTypes: true }); | ||
| for (const entry of entries) { | ||
| const relativePath = prefix ? `${prefix}/${entry.name}` : entry.name; | ||
| if (entry.isDirectory()) { | ||
| walkDir(join(dir, entry.name), relativePath); | ||
| } else if (entry.name.endsWith(".json") || entry.name.endsWith(".md")) { | ||
| // Return absolute or relative path based on parameter | ||
| const path = absolute ? join(this.cacheDir, relativePath) : relativePath; | ||
| files.push(path); | ||
| } | ||
| } | ||
| }; | ||
|
|
||
| walkDir(this.cacheDir); | ||
| return files; | ||
| } | ||
|
|
||
| /** | ||
| * Get cache statistics | ||
| * @returns Object with count and total size | ||
| */ | ||
| async getCacheStats(): Promise<{ count: number; totalSizeBytes: number }> { | ||
| let count = 0; | ||
| let totalSizeBytes = 0; | ||
|
|
||
| const walkDir = (dir: string) => { | ||
| if (!existsSync(dir)) return; | ||
| const entries = readdirSync(dir, { withFileTypes: true }); | ||
| for (const entry of entries) { | ||
| const filePath = join(dir, entry.name); | ||
| if (entry.isDirectory()) { | ||
| walkDir(filePath); | ||
| } else { | ||
| count++; | ||
| const stats = statSync(filePath); | ||
| totalSizeBytes += stats.size; | ||
| } | ||
| } | ||
| }; | ||
|
|
||
| walkDir(this.cacheDir); | ||
| return { count, totalSizeBytes }; | ||
| } | ||
|
|
||
| // ============================================ | ||
| // Raw File Management (for non-JSON content) | ||
| // ============================================ | ||
|
|
||
| /** | ||
| * Put a raw file in the cache (for non-JSON content like markdown, text, etc.) | ||
| * Expiration is checked via file modification time | ||
| * @param relativePath - Relative path within cache directory | ||
| * @param content - Raw file content (string) | ||
| * @param _ttl - TTL string (expiration checked via file mtime) | ||
| */ | ||
| async putRawFile(relativePath: string, content: string, _ttl: TTLString): Promise<void> { | ||
| await this.ensureDirs(); | ||
| const filePath = this.getCacheFilePath(relativePath); | ||
|
|
||
| // Ensure parent directory exists | ||
| const dir = dirname(filePath); | ||
| if (!existsSync(dir)) { | ||
| mkdirSync(dir, { recursive: true }); | ||
| } | ||
|
|
||
| await writeFile(filePath, content); | ||
| this.logger.debug(`Raw file written: ${filePath}`); | ||
| } | ||
|
|
||
| /** | ||
| * Get a raw file from cache (returns null if not found or expired) | ||
| * Expiration is checked based on file modification time | ||
| * @param relativePath - Relative path within cache directory | ||
| * @param ttl - TTL string to check expiration | ||
| * @returns Raw file content or null | ||
| */ | ||
| async getRawFile(relativePath: string, ttl: TTLString): Promise<string | null> { | ||
| const filePath = this.getCacheFilePath(relativePath); | ||
| const ttlMs = this.parseTTL(ttl); | ||
|
|
||
| if (this.isCacheFileExpired(filePath, ttlMs)) { | ||
| return null; | ||
| } | ||
|
|
||
| try { | ||
| const content = await readFile(filePath); | ||
| return content; | ||
| } catch { | ||
| return null; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The Storage class consistently uses synchronous file system methods (existsSync, mkdirSync, statSync, readdirSync, etc.). While the public methods are async, the underlying implementation is blocking. For better performance and consistency with modern Node.js practices, consider refactoring these calls to their asynchronous counterparts from node:fs/promises (e.g., fs.promises.stat, fs.promises.mkdir).
There was a problem hiding this comment.
Pull request overview
This PR extracts shared utility modules into a new @genesis-tools/utils package (built with tsup) and adds shims to preserve existing @app/utils/* import paths.
Changes:
- Added a new
packages/utilsworkspace with build config, exports map, and extracted utility sources (storage/diff/formatting/path/rate-limit/logger). - Updated root TS path aliases to resolve
@genesis-tools/utilswithin the monorepo. - Replaced
src/utils/storageandsrc/utils/diffimplementations with re-export shims.
Reviewed changes
Copilot reviewed 17 out of 19 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| tsconfig.json | Adds path aliases for @genesis-tools/utils to support monorepo-local development. |
| src/utils/storage/index.ts | Replaces storage module with a shim re-exporting from the new utils package. |
| src/utils/diff.ts | Replaces diff module with a shim re-exporting from the new utils package. |
| packages/utils/tsup.config.ts | Adds tsup multi-entry build setup for the extracted utilities. |
| packages/utils/tsconfig.json | Introduces a TS config for the new utils package. |
| packages/utils/test-import.ts | Adds a manual runtime import sanity-check script against built outputs. |
| packages/utils/src/index.ts | Provides package root exports aggregating core utilities. |
| packages/utils/src/core/storage/storage.ts | Adds extracted Storage implementation. |
| packages/utils/src/core/storage/runtime.ts | Adds Bun/Node runtime abstraction for file I/O. |
| packages/utils/src/core/storage/index.ts | Defines storage subpath exports. |
| packages/utils/src/core/rate-limit.ts | Adds extracted rate limiting helpers. |
| packages/utils/src/core/path.ts | Adds extracted path helpers. |
| packages/utils/src/core/logger.ts | Adds extracted lightweight logger implementation. |
| packages/utils/src/core/formatting.ts | Adds extracted formatting + generic utility functions. |
| packages/utils/src/core/diff.ts | Adds extracted diff implementation with configurable logger/colorizer APIs. |
| packages/utils/package.json | Defines package metadata, exports map, and build scripts for @genesis-tools/utils. |
| packages/utils/bun.lock | Adds lockfile for the new package workspace. |
| ccusage | Updates the ccusage submodule pointer. |
| .claude/plans/regiojet-dashboard.md | Adds an implementation plan document (non-runtime). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const { existsSync } = require('node:fs'); | ||
| return existsSync(path); |
There was a problem hiding this comment.
packages/utils is ESM (\"type\": \"module\"), so require is not available at runtime and this will throw. Replace this with an ESM-safe approach (e.g., a top-level import { existsSync } from \"node:fs\"; or an async dynamic import) and keep fileExists synchronous by using the static import.
| options?: DiffOptions | ||
| ): Promise<void> => { | ||
| const util = new DiffUtil(options); | ||
| return util.showDiff(oldContent, newContent, oldLabel, newLabel); |
There was a problem hiding this comment.
This does not typecheck under strict because oldLabel?: string / newLabel?: string are passed to a method requiring string. Use oldLabel ?? \"old\" and newLabel ?? \"new\" (or change the exported function signature to match the instance method defaults) so the package builds cleanly with TypeScript.
| return util.showDiff(oldContent, newContent, oldLabel, newLabel); | |
| return util.showDiff(oldContent, newContent, oldLabel ?? "old", newLabel ?? "new"); |
src/utils/storage/index.ts
Outdated
| // Re-export from @genesis-tools/utils package | ||
| // This shim maintains backwards compatibility with @app/utils/storage imports | ||
|
|
||
| export { Storage, type TTLString, type StorageLogger } from "@genesis-tools/utils/core/storage/storage"; |
There was a problem hiding this comment.
This shim imports a deep internal path (@genesis-tools/utils/core/storage/storage) that is not declared in @genesis-tools/utils exports, and likely won’t exist as a file in dist with the current tsup multi-entry bundling (dependencies are typically bundled into the entry output). Re-export from a public subpath declared in package.json (e.g. @genesis-tools/utils/storage) or from the root @genesis-tools/utils entry to avoid runtime ERR_PACKAGE_PATH_NOT_EXPORTED / missing module errors.
| export { Storage, type TTLString, type StorageLogger } from "@genesis-tools/utils/core/storage/storage"; | |
| export { Storage, type TTLString, type StorageLogger } from "@genesis-tools/utils/storage"; |
src/utils/diff.ts
Outdated
| type DiffLogger, | ||
| type DiffColorizer, | ||
| type DiffOptions, | ||
| } from "@genesis-tools/utils/core/diff"; |
There was a problem hiding this comment.
Same issue as storage shim: @genesis-tools/utils/core/diff is not an exported subpath in packages/utils/package.json, and may not exist as a standalone file in dist depending on tsup bundling. Re-export from the public subpath (@genesis-tools/utils/diff) or root entry to keep Node ESM resolution working.
| } from "@genesis-tools/utils/core/diff"; | |
| } from "@genesis-tools/utils/diff"; |
src/utils/storage/index.ts
Outdated
| // Re-export from @genesis-tools/utils package | ||
| // This shim maintains backwards compatibility with @app/utils/storage imports | ||
|
|
||
| export { Storage, type TTLString, type StorageLogger } from "@genesis-tools/utils/core/storage/storage"; |
There was a problem hiding this comment.
This shim claims backwards compatibility for @app/utils/storage, but it no longer re-exports withFileLock (previously exported from ./file-lock). If existing callers import withFileLock from @app/utils/storage, this change is a breaking API change. To maintain compatibility, re-export withFileLock here as well (either from the existing local implementation or from the new utils package if it was migrated).
| export { Storage, type TTLString, type StorageLogger } from "@genesis-tools/utils/core/storage/storage"; | |
| export { Storage, type TTLString, type StorageLogger } from "@genesis-tools/utils/core/storage/storage"; | |
| export { withFileLock } from "./file-lock"; |
| * This module provides a simple logging interface that can work with or without pino. | ||
| * When pino is available as a peer dependency, it uses pino for logging. | ||
| * Otherwise, it falls back to console-based logging. | ||
| */ | ||
|
|
||
| export type LogLevel = "trace" | "debug" | "info" | "warn" | "error" | "silent"; | ||
|
|
There was a problem hiding this comment.
The docstring claims the logger uses pino when available, but the implementation below never attempts to import or use pino (it always logs via console.*). Either update the docstring to match the actual behavior, or implement the optional pino-backed logger so the documentation stays accurate.
| * This module provides a simple logging interface that can work with or without pino. | |
| * When pino is available as a peer dependency, it uses pino for logging. | |
| * Otherwise, it falls back to console-based logging. | |
| */ | |
| export type LogLevel = "trace" | "debug" | "info" | "warn" | "error" | "silent"; | |
| * This module provides a simple console-based logging interface with | |
| * configurable log levels and optional prefixes. | |
| */ | |
| export type LogLevel = "trace" | "debug" | "info" | "warn" | "error" | "silent"; |
| @@ -0,0 +1,457 @@ | |||
| import { existsSync, mkdirSync, statSync, unlinkSync, readdirSync } from "node:fs"; | |||
There was a problem hiding this comment.
Several async Storage APIs perform synchronous filesystem operations (existsSync, mkdirSync, statSync, readdirSync, unlinkSync), which can block the event loop under load (especially clearCache, listCacheFiles, and getCacheStats). Consider switching to node:fs/promises equivalents (and/or batching) to keep these operations non-blocking.
| import { existsSync, mkdirSync, statSync, unlinkSync, readdirSync } from "node:fs"; | |
| import { access, mkdir, stat, unlink, readdir } from "node:fs/promises"; |
| async ensureDirs(): Promise<void> { | ||
| if (!existsSync(this.baseDir)) { | ||
| mkdirSync(this.baseDir, { recursive: true }); | ||
| this.logger.debug(`Created directory: ${this.baseDir}`); | ||
| } | ||
|
|
||
| if (!existsSync(this.cacheDir)) { | ||
| mkdirSync(this.cacheDir, { recursive: true }); | ||
| this.logger.debug(`Created directory: ${this.cacheDir}`); | ||
| } | ||
| } |
There was a problem hiding this comment.
Several async Storage APIs perform synchronous filesystem operations (existsSync, mkdirSync, statSync, readdirSync, unlinkSync), which can block the event loop under load (especially clearCache, listCacheFiles, and getCacheStats). Consider switching to node:fs/promises equivalents (and/or batching) to keep these operations non-blocking.
- Fix critical stale cleanup scope bug (Thread #4/#11): project-scoped listing no longer deletes other projects' cache entries - Preserve matchSnippet when merging search results (Thread #19) - Fix rg-only results getting 0 slots when meta returns 20 (Thread #13) - Use rmSync with force flag instead of repetitive try/catch (Thread #1) - Add try/catch fallback for METADATA_VERSION (Thread #3/#6) - Add -- and -F flags to ripgrep commands (Thread #5/#17) - Reuse extractTextFromMessage in rgExtractSnippet (Thread #2) - Add 10MB file size cap + early exit in extraction (Thread #7/#18) - Add error handling for ripgrep functions (Thread #10/#16)
- Fix critical stale cleanup scope bug (Thread #4/#11): project-scoped listing no longer deletes other projects' cache entries - Preserve matchSnippet when merging search results (Thread #19) - Fix rg-only results getting 0 slots when meta returns 20 (Thread #13) - Use rmSync with force flag instead of repetitive try/catch (Thread #1) - Add try/catch fallback for METADATA_VERSION (Thread #3/#6) - Add -- and -F flags to ripgrep commands (Thread #5/#17) - Reuse extractTextFromMessage in rgExtractSnippet (Thread #2) - Add 10MB file size cap + early exit in extraction (Thread #7/#18) - Add error handling for ripgrep functions (Thread #10/#16)
#25) * fix(claude-history): full metadata extraction, rg search, auto-reindex - Read entire JSONL files for metadata (was limited to 50 lines/64KB) - Store full firstPrompt (was truncated to 120 chars) - Index ALL user messages into allUserText field (capped 5000 chars) - Summary/custom-title: latest wins (captures tail of file) - Add ripgrep full-content search fallback (rgSearchFiles/rgExtractSnippet) - Auto-reindex via MD5 hash of lib.ts+cache.ts (METADATA_VERSION) - Clean up stale cache entries for deleted session files - Rename DB from stats-cache.db to index.db - Show match snippets in claude-resume session picker - Search firstPrompt in matchByIdOrName * feat(claude-resume): show conversation excerpts in session picker Adds a second line below each session name showing additional context: summary (if name is from title), first prompt (if name is from summary), or match snippet (if from search). * fix(claude-resume): run metadata + rg search in parallel, merge results Previously rg only ran as fallback when metadata returned 0 results. Now both phases run concurrently via Promise.all. Results are deduped and merged: metadata matches first (ranked by relevance), then rg-only matches with snippets. Catches content in assistant messages and user text past the 5000-char metadata cap. * fix(claude-history): resolve dashed project names, add nerdy stats - extractProjectName: progressively resolve encoded dir against filesystem to recover dashed names (col-fe was showing as "fe") - Cache resolved project names to avoid repeated filesystem lookups - Show detailed stats in session picker: session count, project count, scope, indexed/stale/reindex status * feat(claude-resume): nerdy search diagnostics Show detailed breakdown of where results came from: - Index matches: count + what was searched (name/branch/project/prompt) - Content search: meta hits, rg total hits, overlap, rg-only unique hits * fix(claude-history): address PR #25 review comments - Fix critical stale cleanup scope bug (Thread #4/#11): project-scoped listing no longer deletes other projects' cache entries - Preserve matchSnippet when merging search results (Thread #19) - Fix rg-only results getting 0 slots when meta returns 20 (Thread #13) - Use rmSync with force flag instead of repetitive try/catch (Thread #1) - Add try/catch fallback for METADATA_VERSION (Thread #3/#6) - Add -- and -F flags to ripgrep commands (Thread #5/#17) - Reuse extractTextFromMessage in rgExtractSnippet (Thread #2) - Add 10MB file size cap + early exit in extraction (Thread #7/#18) - Add error handling for ripgrep functions (Thread #10/#16) * fix(claude-history): add cwd to early-exit check, use null for empty allUserText - Include cwd in early-exit condition to avoid skipping cwd extraction - Return null instead of "" for allUserText when no user text found - Anonymize path examples in comments
- Replace duplicate stripAnsi with import from utils/string (#18) - Fix wrapToWidth ANSI-aware truncation (#1/#3/#22) - Add await to recursive executeTool call (#9/#20) - Guard cursor when filtered list is empty (#5/#19) - Use basename() instead of split("/").pop() (#10/#16) - Validate --width NaN input (#14) - Re-display watch message after screen clear (#15)
- Replace duplicate stripAnsi with import from utils/string (#18) - Fix wrapToWidth ANSI-aware truncation (#1/#3/#22) - Add await to recursive executeTool call (#9/#20) - Guard cursor when filtered list is empty (#5/#19) - Use basename() instead of split("/").pop() (#10/#16) - Validate --width NaN input (#14) - Re-display watch message after screen clear (#15)
* fix: clean error formatting and fuzzy match for tools entry point * feat(tools): add discovery, introspection, and search-select modules - discovery.ts: scans src/ for tools, extracts descriptions from README.md - introspect.ts: runs --help on tools and parses Commander output into structured data - search-select.ts: single-select search prompt with onHighlight callback, modeled on search-multiselect * feat(tools): interactive browser with search, README preview, and subcommand explorer * feat(markdown): add render options and CLI flags (--watch, --width, --theme, --no-color) * fix: improve fuzzy matching to handle trailing dashes and segment matching "tools cli-" now correctly matches "markdown-cli" by stripping trailing dashes and checking dash-separated segments. * feat(markdown-cli): add demo subcommand with interactive template gallery * Revert "feat(markdown-cli): add demo subcommand with interactive template gallery" This reverts commit f5c84db. * feat(markdown): implement theme palette system (dark/light/minimal) Wire up the --theme flag to actual color palettes that change mermaid blocks, table borders, and alert colors per theme. * fix: address PR #28 code review feedback - Replace duplicate stripAnsi with import from utils/string (#18) - Fix wrapToWidth ANSI-aware truncation (#1/#3/#22) - Add await to recursive executeTool call (#9/#20) - Guard cursor when filtered list is empty (#5/#19) - Use basename() instead of split("/").pop() (#10/#16) - Validate --width NaN input (#14) - Re-display watch message after screen clear (#15) * fix(markdown-cli/tools): address PR #28 review feedback - Remove stray n8n.json (unrelated to PR scope) - fix(markdown-cli): reject invalid --theme values via Commander .choices() (Thread #23) - fix(tools): guard discoverTools against missing srcDir with existsSync check (Thread #24) - fix(markdown): use display width for emoji/wide chars in wrapToWidth (Thread #25)
genesiscz
force-pushed
the
feat/monorepo
branch
from
0be9391 to
52f7364
Compare
genesiscz
force-pushed
the
feat/monorepo
branch
from
52f7364 to
b450c59
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/utils/package.json`:
- Around line 79-82: The package.json "repository" object currently uses a
placeholder URL containing "your-username"; update the "url" value in the
repository field to the actual GitHub repository URL (replace
"https://github.com/your-username/genesis-tools.git" with the real repo URL) so
the package metadata points to the correct remote. Ensure the repository.url
string matches the actual organization/user and repo name.
In `@packages/utils/src/core/diff.ts`:
- Around line 1-4: detectConflicts() currently compares objects by
JSON.stringify which is order-sensitive and can falsely mark identical objects
(like env) as conflicts; replace the stringified comparison with a deep equality
check (e.g., use Node's util.isDeepStrictEqual or a small recursive deepEqual)
when comparing entries in detectConflicts and the same comparison logic around
the other occurrence (the block referenced by the same comparison logic around
lines 213-220). Update comparisons that currently use JSON.stringify(a) ===
JSON.stringify(b) to call the deep-equal helper (or util.isDeepStrictEqual) for
the relevant symbols (detectConflicts, and the duplicate comparison logic) so
objects with the same keys/values in different orders are treated as equal.
In `@packages/utils/src/core/formatting.ts`:
- Around line 278-289: deepMerge currently uses for...in which iterates
inherited properties and can allow prototype pollution; change the loop to
iterate only own keys (e.g., Object.keys(source) or
Object.getOwnPropertyNames(source)) and explicitly skip dangerous sentinel
property names ("__proto__", "constructor", "prototype") before assigning into
result; keep existing behavior for nested object merging (deepMerge and isObject
references) but ensure you never assign or recurse on those rejected keys.
- Around line 254-261: The withTimeout function leaves the timer pending and
keeps the event loop alive; change timeoutPromise to capture the timer id (const
timer = setTimeout(...)) and ensure you clearTimeout(timer) when the race
settles: store the timer id in the scope of withTimeout and clear it in both
resolution and rejection paths (e.g., attach a finally handler to the raced
result or wrap promise/timeout so that clearTimeout(timer) is called once either
promise or timeout wins) so the timer is always cleared; update the function
named withTimeout and the timeoutPromise creation to use this pattern.
- Around line 97-108: sanitizeOutput currently strips the ESC character (\u001B)
even when removeANSI is false, breaking ANSI sequences; update the second regex
in sanitizeOutput so it does not remove \u001B unless removeANSI was true —
i.e., exclude \u001B from the character class (use ranges like
[\u0000-\u001A\u001C-\u001F\u007F-\u009F]) so sanitizeOutput (and the sanitized
variable) preserves full ANSI sequences unless the removeANSI branch removed
them.
In `@packages/utils/src/core/logger.ts`:
- Around line 1-7: The file-level docstring in packages/utils/src/core/logger.ts
incorrectly claims pino integration and a fallback, while the implementation is
console-only; update the header comment to accurately describe the current
behavior (a lightweight console-based logger that declares pino as a peer
dependency but does not use it), and optionally add a TODO noting that pino
support can be implemented later or remove the unused pino peer dependency from
package metadata; locate the top comment block in logger.ts to make this change
and ensure the docstring matches the actual implementation and exported symbols.
- Around line 134-145: The warn and error logger implementations currently call
console.log which writes to stdout; update the functions named warn and error
(inside the exported logger object that uses shouldLog(logLevel), isTerminal,
colors, prefix, and formatArgs) to call console.warn for warn level and
console.error for error level respectively while preserving the existing checks,
levelPrefix computation, and message construction so output formatting and
conditional logging remain unchanged.
- Around line 30-47: getLogLevel currently honors LOG_SILENT env var but not the
--silent CLI flag; update getLogLevel to check process.argv for "--silent"
(e.g., process.argv.includes("--silent")) and return "silent" if present,
placing that check before the verbose/trace CLI checks so CLI --silent takes
precedence; modify the function that currently inspects process.argv
(getLogLevel) to include this new condition.
- Around line 106-113: Remove the empty try-catch that attempts to load "chalk"
(the no-op block between the comments) in logger.ts; delete the dead code block
and either leave a single concise comment explaining that dynamic imports aren't
used here or move the existing explanation above the logger/colorizer
initialization, ensuring no behavioral change and satisfying linters about empty
blocks.
In `@packages/utils/src/core/path.ts`:
- Around line 22-28: The function resolvePathWithTilde currently replaces a
leading "~" with os.homedir() but doesn't normalize/resolve the remainder,
causing inconsistent handling of segments like "~/foo/../bar"; update
resolvePathWithTilde to only replace the leading tilde (e.g., check
path.startsWith("~") and build a newPath as os.homedir() + path.slice(1) or use
a leading-tilde regex) and then call pathUtils.resolve(newPath) so both tilde
and non-tilde branches return a fully resolved, normalized path; reference
resolvePathWithTilde, os.homedir(), and pathUtils.resolve when making the
change.
In `@packages/utils/src/core/rate-limit.ts`:
- Around line 51-63: Parsed header values from error.headers["retry-after"] and
error.headers["x-ratelimit-reset"] must be validated before use; update the
logic in functions that compute headerDelay (and the fallback that uses
headerDelay ?? exponentialDelay) to check that parseInt(...) yields a finite
positive number (e.g., Number.isFinite(parsed) && parsed > 0) and otherwise
ignore the header and fall back to exponentialDelay; apply the same validation
to the x-ratelimit-reset parsing (compute resetTime and delay only if parsed
values are valid) and keep the existing 60s cap logic when a valid delay is
present.
- Around line 140-153: The current createRateLimitedCaller uses a shared
lastCallTime that can be read concurrently, so multiple invocations can slip
through; fix it by serializing invocations with a promise chain or mutex: add a
module-scoped or closure-scoped "lastPromise" (or lock) that each call awaits
before computing timeSinceLastCall and calling sleep, update lastCallTime only
while holding the lock/serial chain, then call withRetry(fn, options) and
resolve the chain; reference createRateLimitedCaller, lastCallTime,
lastPromise/lock, sleep, and withRetry to implement the serialized queuing so
callers cannot run the timing check concurrently.
In `@packages/utils/src/core/storage/runtime.ts`:
- Around line 31-38: The writeFile function is using optional chaining
(Bun?.write) despite the isBun guard, which defeats type narrowing; update
writeFile to call Bun.write without optional chaining after confirming isBun —
e.g., inside the if (isBun) block reference Bun.write directly (or capture
Bun.write into a local const) and await that, so the code no longer uses Bun?
and satisfies the type checker while preserving the node fallback via
import("node:fs/promises") and writeFile.
- Around line 19-25: The readFile function uses optional chaining
(Bun?.file(path).text()) which can produce undefined even though isBun was
checked; update the Bun usage to a non-null assertion or explicit runtime check
to satisfy TypeScript. Specifically, in readFile replace Bun?.file(path).text()
with either Bun!.file(path).text() or guard on Bun (e.g., if (!Bun) throw)
before calling file so the return value from Bun.file(path).text() is treated as
a non-undefined string; keep references to the readFile function and the
Bun/isBun checks when making the change.
- Around line 43-46: Replace the CommonJS require in fileExists with an ESM
import: move to a top-level ESM import like `import { existsSync } from
"node:fs";` and have fileExists continue to return existsSync(path);
(alternatively, if you prefer dynamic import, make fileExists async and use
`const { existsSync } = await import("node:fs");`); update the function
signature accordingly if you choose the async approach.
In `@packages/utils/src/core/storage/storage.ts`:
- Around line 148-153: When walking the config path in the loop that iterates
over keys (variables: keys, current, k) treat null and arrays as terminal values
so they get overwritten with an object; update the guard so it not only checks
typeof current[k] !== "object" but also treats current[k] === null and
Array.isArray(current[k]) as reasons to replace the value. In other words, in
the loop around keys/current/k, change the condition that decides to set
current[k] = {} to include null and arrays (use a check like current[k] === null
|| Array.isArray(current[k]) or combine with the existing typeof check) so
existing null or array values don’t leak through and crash on the next
assignment.
- Around line 39-43: The constructor and cache file methods currently join raw
inputs into filesystem paths (constructor: toolName ->
baseDir/cacheDir/configPath; methods at/around putCacheFile and getCachePath)
allowing path traversal; fix by sanitizing toolName (e.g., restrict to a safe
token: use path.basename or validate with a regex like /^[A-Za-z0-9_-]+$/ and
throw on invalid/empty names) and by resolving+validating any file path created
under baseDir (use path.resolve(join(baseDir, userPath)) then assert the
resolved path startsWith(baseDir) before using it, throwing an error if it
escapes). Apply this pattern in the constructor and in the cache-related
functions referenced (e.g., putCacheFile, getCachePath) so no user input can
break out of ~/.genesis-tools/<toolName>.
In `@packages/utils/test-import.ts`:
- Around line 1-29: Replace the current console-only smoke test with real
assertions so failures break the build: call functions like
formatDuration(125000) and formatTokens(15000) and assert their outputs match
expected strings, assert Storage("test-tool").getBaseDir() and
StorageSubpath("test-subpath").getCacheDir() return non-empty strings (or
specific expected paths), assert debounce(...) returns a function and that
createLogger({level:"info"}).level === "info"; throw or exit non-zero on any
mismatch so the script fails on wrong behavior rather than only logging values.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9ffa1a9d-5320-493c-82cf-bc0168fed9f0
⛔ Files ignored due to path filters (1)
packages/utils/bun.lockis excluded by!**/*.lock
📒 Files selected for processing (17)
.claude/plans/regiojet-dashboard.mdccusagepackages/utils/package.jsonpackages/utils/src/core/diff.tspackages/utils/src/core/formatting.tspackages/utils/src/core/logger.tspackages/utils/src/core/path.tspackages/utils/src/core/rate-limit.tspackages/utils/src/core/storage/index.tspackages/utils/src/core/storage/runtime.tspackages/utils/src/core/storage/storage.tspackages/utils/src/index.tspackages/utils/test-import.tspackages/utils/tsconfig.jsonpackages/utils/tsup.config.tssrc/utils/storage/index.tstsconfig.json
📜 Review details
🧰 Additional context used
📓 Path-based instructions (4)
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Usebun runto execute TypeScript files directly without a build step
Place general-purpose helper functions insrc/utils/instead of tool directories
Never add// src/path/to/file.tsas the first line of files
Don't add comments that restate what the code already says (e.g., obvious descriptions of function calls)
Always use block form with braces forifstatements, even for early returns; never use one-lineifstatements
Add an empty line beforeifstatements unless the preceding line is a variable declaration used by thatif
Add an empty line after closing}unless followed byelse,catch,finally, or another}
Noas anytype assertions; use proper type narrowing, type guards, or explicit interfaces instead
Use discriminant checks (e.g.,entity.className === "User") when working with union types instead of type assertions
Prefererror: errovererror: err instanceof Error ? err.message : String(err)when the error field accepts unknown type
Files:
packages/utils/src/core/storage/runtime.tspackages/utils/tsup.config.tspackages/utils/src/core/path.tspackages/utils/src/core/rate-limit.tspackages/utils/src/index.tspackages/utils/test-import.tspackages/utils/src/core/storage/index.tssrc/utils/storage/index.tspackages/utils/src/core/formatting.tspackages/utils/src/core/diff.tspackages/utils/src/core/storage/storage.tspackages/utils/src/core/logger.ts
src/**/index.ts
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/index.ts: Usecommanderfor parsing command-line arguments with subcommands and options
Support multiple output destinations (file, clipboard, stdout) and useclipboardyfor clipboard operations
Tool discovery checks for directories withindex.tsorindex.tsx(tool name = directory name) and standalone.tsor.tsxfiles (tool name = filename without extension)
Respect--silentand--verboseflags in tool output handling
Files:
src/utils/storage/index.ts
src/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.ts: Use@clack/promptsfor interactive user prompts (preferred over@inquirer/prompts)
UseBun.spawn()for executing external commands and always check exit codes with meaningful error messages
Use Node.jspathmodule for cross-platform path handling and resolve relative paths to absolute usingresolve()
Use Bun's native file APIs (Bun.write()) for better performance instead of other file writing methods
Use centralized logging with pino logger outputting to/logs/directory organized by date
Handle user cancellation from prompts gracefully
Provide sensible defaults and suggestions in user prompts
Strip ANSI codes fromchalkcolored output for non-TTY environments
Files:
src/utils/storage/index.ts
src/utils/**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Check if a utility is general-purpose (usable by other tools) before deciding to place it in
src/utils/
Files:
src/utils/storage/index.ts
🧠 Learnings (24)
📓 Common learnings
Learnt from: CR
Repo: genesiscz/GenesisTools PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-20T01:35:08.233Z
Learning: Applies to **/*.ts : Place general-purpose helper functions in `src/utils/` instead of tool directories
Learnt from: CR
Repo: genesiscz/GenesisTools PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-20T01:35:08.233Z
Learning: Applies to src/utils/**/*.ts : Check if a utility is general-purpose (usable by other tools) before deciding to place it in `src/utils/`
Learnt from: genesiscz
Repo: genesiscz/GenesisTools PR: 82
File: src/claude/lib/usage/dashboard-config.ts:24-25
Timestamp: 2026-03-09T13:39:43.059Z
Learning: GenesisTools (genesiscz/GenesisTools) is a personal CLI tool, not a multi-tenant service. Do not suggest data migration paths or backward-compatibility shims for simple default value changes in configuration files (e.g., dashboard-config.ts). The TUI provides interactive controls (e.g., the `i` key to cycle poll intervals at runtime), so users can adjust settings without needing automated migrations. Avoid overengineering suggestions in this context.
📚 Learning: 2026-02-20T01:35:08.233Z
Learnt from: CR
Repo: genesiscz/GenesisTools PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-20T01:35:08.233Z
Learning: Applies to **/*.ts : Place general-purpose helper functions in `src/utils/` instead of tool directories
Applied to files:
packages/utils/tsconfig.jsonpackages/utils/tsup.config.tspackages/utils/src/core/path.tspackages/utils/package.jsonpackages/utils/src/index.tspackages/utils/test-import.tssrc/utils/storage/index.tspackages/utils/src/core/formatting.tstsconfig.jsonpackages/utils/src/core/storage/storage.ts
📚 Learning: 2026-02-20T01:35:08.233Z
Learnt from: CR
Repo: genesiscz/GenesisTools PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-20T01:35:08.233Z
Learning: Applies to src/**/*.ts : Provide sensible defaults and suggestions in user prompts
Applied to files:
packages/utils/tsconfig.jsonpackages/utils/tsup.config.tspackages/utils/src/core/diff.tstsconfig.json
📚 Learning: 2026-02-20T01:35:08.233Z
Learnt from: CR
Repo: genesiscz/GenesisTools PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-20T01:35:08.233Z
Learning: Applies to **/*.ts : Never add `// src/path/to/file.ts` as the first line of files
Applied to files:
packages/utils/tsconfig.jsontsconfig.json
📚 Learning: 2026-02-20T01:35:08.233Z
Learnt from: CR
Repo: genesiscz/GenesisTools PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-20T01:35:08.233Z
Learning: Applies to **/*.ts : Use `bun run` to execute TypeScript files directly without a build step
Applied to files:
packages/utils/tsconfig.jsonpackages/utils/src/core/storage/runtime.tspackages/utils/tsup.config.ts
📚 Learning: 2026-02-20T01:35:08.233Z
Learnt from: CR
Repo: genesiscz/GenesisTools PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-20T01:35:08.233Z
Learning: Applies to src/**/index.ts : Tool discovery checks for directories with `index.ts` or `index.tsx` (tool name = directory name) and standalone `.ts` or `.tsx` files (tool name = filename without extension)
Applied to files:
packages/utils/tsconfig.jsonpackages/utils/tsup.config.tspackages/utils/test-import.tstsconfig.json
📚 Learning: 2026-02-20T01:35:08.233Z
Learnt from: CR
Repo: genesiscz/GenesisTools PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-20T01:35:08.233Z
Learning: Applies to **/*.ts : No `as any` type assertions; use proper type narrowing, type guards, or explicit interfaces instead
Applied to files:
packages/utils/tsconfig.json
📚 Learning: 2026-02-20T01:35:08.233Z
Learnt from: CR
Repo: genesiscz/GenesisTools PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-20T01:35:08.233Z
Learning: Applies to src/**/index.ts : Respect `--silent` and `--verbose` flags in tool output handling
Applied to files:
packages/utils/tsconfig.jsonpackages/utils/src/core/logger.ts
📚 Learning: 2026-02-20T01:35:08.233Z
Learnt from: CR
Repo: genesiscz/GenesisTools PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-20T01:35:08.233Z
Learning: Applies to src/**/*.ts : Use Node.js `path` module for cross-platform path handling and resolve relative paths to absolute using `resolve()`
Applied to files:
packages/utils/tsconfig.jsonpackages/utils/src/core/storage/runtime.tspackages/utils/src/core/path.tssrc/utils/storage/index.tstsconfig.json
📚 Learning: 2026-02-20T01:35:08.233Z
Learnt from: CR
Repo: genesiscz/GenesisTools PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-20T01:35:08.233Z
Learning: Applies to src/**/*.ts : Use Bun's native file APIs (`Bun.write()`) for better performance instead of other file writing methods
Applied to files:
packages/utils/src/core/storage/runtime.tspackages/utils/src/core/storage/index.tspackages/utils/src/core/storage/storage.ts
📚 Learning: 2026-02-20T01:35:08.233Z
Learnt from: CR
Repo: genesiscz/GenesisTools PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-20T01:35:08.233Z
Learning: Applies to src/**/*.ts : Use `Bun.spawn()` for executing external commands and always check exit codes with meaningful error messages
Applied to files:
packages/utils/src/core/storage/runtime.ts
📚 Learning: 2026-02-24T15:32:44.925Z
Learnt from: genesiscz
Repo: genesiscz/GenesisTools PR: 54
File: src/github/lib/review-output.ts:18-20
Timestamp: 2026-02-24T15:32:44.925Z
Learning: In TypeScript files, do not require a blank line between the opening brace of a function and the first statement if the first statement is the if statement immediately after the signature. The blank-line rule applies to separating an if from unrelated preceding code within the same block, not to spacing after the function opening brace. Apply this rule to all TS functions across the codebase.
Applied to files:
packages/utils/src/core/storage/runtime.tspackages/utils/tsup.config.tspackages/utils/src/core/path.tspackages/utils/src/core/rate-limit.tspackages/utils/src/index.tspackages/utils/test-import.tspackages/utils/src/core/storage/index.tssrc/utils/storage/index.tspackages/utils/src/core/formatting.tspackages/utils/src/core/diff.tspackages/utils/src/core/storage/storage.tspackages/utils/src/core/logger.ts
📚 Learning: 2026-03-09T13:39:43.059Z
Learnt from: genesiscz
Repo: genesiscz/GenesisTools PR: 82
File: src/claude/lib/usage/dashboard-config.ts:24-25
Timestamp: 2026-03-09T13:39:43.059Z
Learning: GenesisTools (genesiscz/GenesisTools) is a personal CLI tool, not a multi-tenant service. Do not suggest data migration paths or backward-compatibility shims for simple default value changes in configuration files (e.g., dashboard-config.ts). The TUI provides interactive controls (e.g., the `i` key to cycle poll intervals at runtime), so users can adjust settings without needing automated migrations. Avoid overengineering suggestions in this context.
Applied to files:
packages/utils/package.jsonpackages/utils/src/index.ts.claude/plans/regiojet-dashboard.mdtsconfig.json
📚 Learning: 2026-02-20T01:35:08.233Z
Learnt from: CR
Repo: genesiscz/GenesisTools PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-20T01:35:08.233Z
Learning: Applies to src/utils/**/*.ts : Check if a utility is general-purpose (usable by other tools) before deciding to place it in `src/utils/`
Applied to files:
packages/utils/package.jsonpackages/utils/src/index.tspackages/utils/test-import.tssrc/utils/storage/index.tspackages/utils/src/core/formatting.ts
📚 Learning: 2026-03-09T13:14:03.722Z
Learnt from: genesiscz
Repo: genesiscz/GenesisTools PR: 81
File: src/github/commands/get.ts:209-212
Timestamp: 2026-03-09T13:14:03.722Z
Learning: In the GenesisTools repo (genesiscz/GenesisTools), there is no CI formatter (e.g., Prettier/Biome) configured or enforced. Do not flag formatting mismatches based on static analysis tool annotations (e.g., GitHub Actions CI formatter warnings) in this repository, as they are not representative of actual enforced formatting rules. TypeScript files under src/ should be reviewed for logical correctness and consistency with existing code patterns instead.
Applied to files:
packages/utils/package.jsonpackages/utils/src/index.tstsconfig.json
📚 Learning: 2026-02-27T05:49:10.156Z
Learnt from: genesiscz
Repo: genesiscz/GenesisTools PR: 64
File: src/utils/ink/components/workflow-progress.tsx:18-21
Timestamp: 2026-02-27T05:49:10.156Z
Learning: Files in `src/utils/ink/` are pre-existing shared component library files copied as-is. They are not currently used by active code (e.g., the usage TUI dashboard) and will be addressed separately when those components are needed. Do not flag import errors or missing dependencies in these files during reviews.
Applied to files:
packages/utils/src/index.tspackages/utils/test-import.ts
📚 Learning: 2026-02-20T00:52:27.023Z
Learnt from: genesiscz
Repo: genesiscz/GenesisTools PR: 31
File: src/ask/utils/helpers.ts:3-3
Timestamp: 2026-02-20T00:52:27.023Z
Learning: In all TypeScript source files under src, prefer using picocolors for colored terminal output in new code. Picocolors is smaller and faster than chalk, so adopt it for CLI output coloring and avoid adding chalk in new code paths unless there is a compelling compatibility reason.
Applied to files:
src/utils/storage/index.ts
📚 Learning: 2026-02-24T15:32:37.494Z
Learnt from: genesiscz
Repo: genesiscz/GenesisTools PR: 54
File: src/github/lib/output.ts:109-113
Timestamp: 2026-02-24T15:32:37.494Z
Learning: In TypeScript files under src/, do not require a leading blank line before an if statement that is the first statement inside a function body (immediately after the function signature). The blank line rule should only apply to if statements that come after other statements within the function body. Apply this guideline consistently across TS files in src to reduce unnecessary vertical whitespace and keep concise function bodies.
Applied to files:
src/utils/storage/index.ts
📚 Learning: 2026-03-09T13:13:58.786Z
Learnt from: genesiscz
Repo: genesiscz/GenesisTools PR: 81
File: src/github/commands/get.ts:209-212
Timestamp: 2026-03-09T13:13:58.786Z
Learning: In the GenesisTools repo (genesiscz/GenesisTools), do not treat CI formatter warnings as enforceable formatting rules for TypeScript files under src/. Focus reviews on logical correctness and consistency with existing code patterns. For files under src (e.g., src/github/commands/get.ts), prioritize code structure, readability, naming, correctness, and adherence to project conventions over automated formatting warnings from CI tools.
Applied to files:
src/utils/storage/index.ts
📚 Learning: 2026-02-17T01:37:29.373Z
Learnt from: genesiscz
Repo: genesiscz/GenesisTools PR: 29
File: .claude/plans/2026-02-16-ClaudeHistory-SummarizeMemorize-Implementation.md:11-15
Timestamp: 2026-02-17T01:37:29.373Z
Learning: Do not apply markdownlint to files under the .claude/ directory. This exemption covers design documents, plans, and archived content within .claude/ (e.g., .claude/plans/.../*.md). Treat these files as exempt from markdownlint rules to avoid false positives on non-standard or archival content.
Applied to files:
.claude/plans/regiojet-dashboard.md
📚 Learning: 2026-02-20T01:35:08.233Z
Learnt from: CR
Repo: genesiscz/GenesisTools PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-20T01:35:08.233Z
Learning: Applies to src/**/*.ts : Use centralized logging with pino logger outputting to `/logs/` directory organized by date
Applied to files:
packages/utils/src/core/logger.ts
📚 Learning: 2026-02-20T00:52:33.267Z
Learnt from: genesiscz
Repo: genesiscz/GenesisTools PR: 31
File: src/ask/utils/helpers.ts:3-3
Timestamp: 2026-02-20T00:52:33.267Z
Learning: Applies to src/**/*.ts: Use `picocolors` for colored terminal output in new code. Picocolors is preferred over chalk for its smaller size and better performance.
Applied to files:
packages/utils/src/core/logger.ts
📚 Learning: 2026-02-24T15:39:24.751Z
Learnt from: genesiscz
Repo: genesiscz/GenesisTools PR: 54
File: src/github/lib/review-output.ts:182-210
Timestamp: 2026-02-24T15:39:24.751Z
Learning: The src/github/ tool uses chalk exclusively for colored terminal output across all 11 files. Do not suggest replacing chalk with picocolors in src/github/ files, as chalk is the established standard for this tool.
Applied to files:
packages/utils/src/core/logger.ts
📚 Learning: 2026-02-20T01:35:08.233Z
Learnt from: CR
Repo: genesiscz/GenesisTools PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-20T01:35:08.233Z
Learning: Applies to src/**/*.ts : Strip ANSI codes from `chalk` colored output for non-TTY environments
Applied to files:
packages/utils/src/core/logger.ts
🧬 Code graph analysis (5)
packages/utils/src/core/path.ts (1)
packages/utils/src/index.ts (3)
tildeifyPath(50-50)resolvePathWithTilde(50-50)normalizeFilePaths(50-50)
packages/utils/test-import.ts (3)
packages/utils/src/core/storage/storage.ts (1)
Storage(27-465)packages/utils/src/core/formatting.ts (3)
formatDuration(45-61)formatTokens(26-33)debounce(195-205)packages/utils/src/core/logger.ts (2)
logger(167-167)createLogger(72-147)
packages/utils/src/core/diff.ts (1)
packages/utils/src/core/storage/runtime.ts (1)
writeFile(31-38)
packages/utils/src/core/storage/storage.ts (2)
packages/utils/src/core/logger.ts (1)
logger(167-167)packages/utils/src/core/storage/runtime.ts (2)
readFile(19-25)writeFile(31-38)
packages/utils/src/core/logger.ts (1)
packages/utils/src/index.ts (7)
LogLevel(46-46)getLogLevel(43-43)LoggerOptions(45-45)Logger(44-44)createLogger(41-41)createNoopLogger(42-42)logger(47-47)
🪛 LanguageTool
.claude/plans/regiojet-dashboard.md
[uncategorized] ~94-~94: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...y - Color intensity = trip count - Show full year view ### Additional Stats (Cards/Metri...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
🪛 markdownlint-cli2 (0.21.0)
.claude/plans/regiojet-dashboard.md
[warning] 9-9: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 10-10: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 10-10: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 24-24: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 25-25: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 25-25: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 35-35: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 36-36: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
[warning] 45-45: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 51-51: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 57-57: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 62-62: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 68-68: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 72-72: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 77-77: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 82-82: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 86-86: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 90-90: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 96-96: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 108-108: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 109-109: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 109-109: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
[warning] 167-167: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 191-191: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 274-274: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 294-294: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 296-296: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
[warning] 302-302: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 308-308: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
[warning] 315-315: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
🔇 Additional comments (10)
.claude/plans/regiojet-dashboard.md (1)
129-129: No changes needed. The import path@app/utils/storageis correct and will continue to work as a re-export shim from the new@genesis-tools/utilspackage. This maintains backwards compatibility within the application.ccusage (1)
1-1: Verify the new ccusage submodule before merging.This commit introduces
ccusageas a new git submodule (SHA0225b9132904ec87b93f3e372426ba0e90c13eb5), but the submodule is not initialized in the repository. Before merging, confirm that:
- This submodule is intentional and necessary for the monorepo structure
- The upstream repository at that commit is stable and trustworthy
- There are no circular dependencies or import path issues with the monorepo extraction
packages/utils/tsconfig.json (1)
1-18: LGTM!The configuration is appropriate for a tsup-based build workflow where
tscis used only for type-checking (noEmit: true) while tsup handles the actual build and declaration generation via itsdts: trueoption.tsconfig.json (1)
37-39: LGTM!Path aliases are correctly configured for both the main barrel import (
@genesis-tools/utils) and deep imports (@genesis-tools/utils/*). Thepackages/utilsfolder has its owntsconfig.jsonfor type-checking.packages/utils/tsup.config.ts (1)
1-22: LGTM!Well-structured tsup configuration with:
- Multiple entry points matching
package.jsonexports- Proper externalization of peer dependencies
- Declaration generation and source maps enabled
src/utils/storage/index.ts (1)
1-4: LGTM!Clean backward-compatibility shim that re-exports from the new package location while preserving existing import paths.
packages/utils/src/core/path.ts (1)
41-94: LGTM - robust input normalization.The
normalizeFilePathsfunction handles multiple input formats gracefully with appropriate fallbacks for JSON arrays, Python-style arrays, and manual parsing. Good defensive coding for MCP tool inputs.packages/utils/package.json (1)
51-54: TypeScript version is current. TypeScript5.9.3is the latest stable version available on npm, so the specified constraint^5.9.3is appropriate.packages/utils/src/core/storage/index.ts (1)
1-2: LGTM.This barrel stays minimal and exposes the storage/runtime split cleanly.
packages/utils/src/index.ts (1)
8-61: LGTM.The root barrel exposes a coherent public surface for the extracted utils package, and the type/value re-exports are explicit.
| "repository": { | ||
| "type": "git", | ||
| "url": "https://github.com/your-username/genesis-tools.git" | ||
| } |
There was a problem hiding this comment.
Update the placeholder repository URL.
The repository URL contains a placeholder your-username that should be updated to match the actual repository.
🔧 Proposed fix
"repository": {
"type": "git",
- "url": "https://github.com/your-username/genesis-tools.git"
+ "url": "https://github.com/genesiscz/GenesisTools.git"
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "repository": { | |
| "type": "git", | |
| "url": "https://github.com/your-username/genesis-tools.git" | |
| } | |
| "repository": { | |
| "type": "git", | |
| "url": "https://github.com/genesiscz/GenesisTools.git" | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/utils/package.json` around lines 79 - 82, The package.json
"repository" object currently uses a placeholder URL containing "your-username";
update the "url" value in the repository field to the actual GitHub repository
URL (replace "https://github.com/your-username/genesis-tools.git" with the real
repo URL) so the package metadata points to the correct remote. Ensure the
repository.url string matches the actual organization/user and repo name.
| import { spawn } from "node:child_process"; | ||
| import { unlink, writeFile } from "node:fs/promises"; | ||
| import { tmpdir } from "node:os"; | ||
| import { join } from "node:path"; |
There was a problem hiding this comment.
Use deep equality in detectConflicts().
JSON.stringify() is order-sensitive for object keys, so semantically identical objects like env can be flagged as conflicts purely because they were constructed in a different order.
Suggested fix
import { spawn } from "node:child_process";
import { unlink, writeFile } from "node:fs/promises";
import { tmpdir } from "node:os";
import { join } from "node:path";
+import { isDeepStrictEqual } from "node:util";
@@
- if (JSON.stringify(oldValue) !== JSON.stringify(newValue)) {
+ if (!isDeepStrictEqual(oldValue, newValue)) {
differences.push(field);
}Also applies to: 213-220
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/utils/src/core/diff.ts` around lines 1 - 4, detectConflicts()
currently compares objects by JSON.stringify which is order-sensitive and can
falsely mark identical objects (like env) as conflicts; replace the stringified
comparison with a deep equality check (e.g., use Node's util.isDeepStrictEqual
or a small recursive deepEqual) when comparing entries in detectConflicts and
the same comparison logic around the other occurrence (the block referenced by
the same comparison logic around lines 213-220). Update comparisons that
currently use JSON.stringify(a) === JSON.stringify(b) to call the deep-equal
helper (or util.isDeepStrictEqual) for the relevant symbols (detectConflicts,
and the duplicate comparison logic) so objects with the same keys/values in
different orders are treated as equal.
| export function sanitizeOutput(text: string, removeANSI: boolean = false): string { | ||
| let sanitized = text; | ||
|
|
||
| if (removeANSI) { | ||
| // Remove ANSI escape codes | ||
| // biome-ignore lint/suspicious/noControlCharactersInRegex: ANSI escape codes use control chars | ||
| sanitized = sanitized.replace(/\x1b\[[0-9;]*m/g, ""); | ||
| } | ||
|
|
||
| // Remove other potentially problematic characters | ||
| // biome-ignore lint/suspicious/noControlCharactersInRegex: intentionally stripping control chars | ||
| sanitized = sanitized.replace(/[\u0000-\u001F\u007F-\u009F]/g, ""); |
There was a problem hiding this comment.
Don't strip ESC unless you're removing the whole ANSI sequence.
When removeANSI is false, the second regex still removes \u001B, so colored output becomes fragments like [31m... instead of staying intact or being fully sanitized.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/utils/src/core/formatting.ts` around lines 97 - 108, sanitizeOutput
currently strips the ESC character (\u001B) even when removeANSI is false,
breaking ANSI sequences; update the second regex in sanitizeOutput so it does
not remove \u001B unless removeANSI was true — i.e., exclude \u001B from the
character class (use ranges like [\u0000-\u001A\u001C-\u001F\u007F-\u009F]) so
sanitizeOutput (and the sanitized variable) preserves full ANSI sequences unless
the removeANSI branch removed them.
| export async function withTimeout<T>(promise: Promise<T>, timeoutMs: number, timeoutError?: Error): Promise<T> { | ||
| const timeoutPromise = new Promise<never>((_, reject) => { | ||
| setTimeout(() => { | ||
| reject(timeoutError || new Error(`Operation timed out after ${timeoutMs}ms`)); | ||
| }, timeoutMs); | ||
| }); | ||
|
|
||
| return Promise.race([promise, timeoutPromise]); |
There was a problem hiding this comment.
Clear the timeout once Promise.race() settles.
The pending timer keeps the event loop alive until timeoutMs elapses even when the wrapped promise resolves immediately, which is noticeable in short-lived CLI commands.
Suggested fix
export async function withTimeout<T>(promise: Promise<T>, timeoutMs: number, timeoutError?: Error): Promise<T> {
- const timeoutPromise = new Promise<never>((_, reject) => {
- setTimeout(() => {
+ let timeoutId: ReturnType<typeof setTimeout> | undefined;
+
+ const timeoutPromise = new Promise<never>((_, reject) => {
+ timeoutId = setTimeout(() => {
reject(timeoutError || new Error(`Operation timed out after ${timeoutMs}ms`));
}, timeoutMs);
});
- return Promise.race([promise, timeoutPromise]);
+ try {
+ return await Promise.race([promise, timeoutPromise]);
+ } finally {
+ if (timeoutId !== undefined) {
+ clearTimeout(timeoutId);
+ }
+ }
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/utils/src/core/formatting.ts` around lines 254 - 261, The
withTimeout function leaves the timer pending and keeps the event loop alive;
change timeoutPromise to capture the timer id (const timer = setTimeout(...))
and ensure you clearTimeout(timer) when the race settles: store the timer id in
the scope of withTimeout and clear it in both resolution and rejection paths
(e.g., attach a finally handler to the raced result or wrap promise/timeout so
that clearTimeout(timer) is called once either promise or timeout wins) so the
timer is always cleared; update the function named withTimeout and the
timeoutPromise creation to use this pattern.
| export function deepMerge<T extends Record<string, unknown>>(target: T, source: Partial<T>): T { | ||
| const result = { ...target }; | ||
|
|
||
| for (const key in source) { | ||
| if (source[key] && isObject(source[key]) && isObject(result[key])) { | ||
| result[key] = deepMerge( | ||
| result[key] as Record<string, unknown>, | ||
| source[key] as Record<string, unknown> | ||
| ) as T[Extract<keyof T, string>]; | ||
| } else if (source[key] !== undefined) { | ||
| result[key] = source[key] as T[Extract<keyof T, string>]; | ||
| } |
There was a problem hiding this comment.
Harden deepMerge() against prototype pollution.
for...in walks inherited keys, and writing __proto__, constructor, or prototype into result lets hostile input mutate the returned object's prototype. Iterate own keys only and reject those sentinel names.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/utils/src/core/formatting.ts` around lines 278 - 289, deepMerge
currently uses for...in which iterates inherited properties and can allow
prototype pollution; change the loop to iterate only own keys (e.g.,
Object.keys(source) or Object.getOwnPropertyNames(source)) and explicitly skip
dangerous sentinel property names ("__proto__", "constructor", "prototype")
before assigning into result; keep existing behavior for nested object merging
(deepMerge and isObject references) but ensure you never assign or recurse on
those rejected keys.
| export async function writeFile(path: string, content: string): Promise<void> { | ||
| if (isBun) { | ||
| await Bun?.write(path, content); | ||
| return; | ||
| } | ||
| const { writeFile } = await import("node:fs/promises"); | ||
| await writeFile(path, content, "utf-8"); | ||
| } |
There was a problem hiding this comment.
Apply the same fix for writeFile.
Same type narrowing issue with optional chaining after the isBun guard.
🛡️ Proposed fix
export async function writeFile(path: string, content: string): Promise<void> {
if (isBun) {
- await Bun?.write(path, content);
+ await Bun!.write(path, content);
return;
}
const { writeFile } = await import("node:fs/promises");
await writeFile(path, content, "utf-8");
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export async function writeFile(path: string, content: string): Promise<void> { | |
| if (isBun) { | |
| await Bun?.write(path, content); | |
| return; | |
| } | |
| const { writeFile } = await import("node:fs/promises"); | |
| await writeFile(path, content, "utf-8"); | |
| } | |
| export async function writeFile(path: string, content: string): Promise<void> { | |
| if (isBun) { | |
| await Bun!.write(path, content); | |
| return; | |
| } | |
| const { writeFile } = await import("node:fs/promises"); | |
| await writeFile(path, content, "utf-8"); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/utils/src/core/storage/runtime.ts` around lines 31 - 38, The
writeFile function is using optional chaining (Bun?.write) despite the isBun
guard, which defeats type narrowing; update writeFile to call Bun.write without
optional chaining after confirming isBun — e.g., inside the if (isBun) block
reference Bun.write directly (or capture Bun.write into a local const) and await
that, so the code no longer uses Bun? and satisfies the type checker while
preserving the node fallback via import("node:fs/promises") and writeFile.
| export function fileExists(path: string): boolean { | ||
| const { existsSync } = require("node:fs"); | ||
| return existsSync(path); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider using ESM dynamic import for consistency.
The rest of the file uses ESM dynamic imports (await import("node:fs/promises")), but fileExists uses CommonJS require. For consistency and to avoid mixing module systems:
♻️ Proposed refactor
-export function fileExists(path: string): boolean {
- const { existsSync } = require("node:fs");
- return existsSync(path);
+import { existsSync } from "node:fs";
+
+export function fileExists(path: string): boolean {
+ return existsSync(path);
}Move the import to the top of the file alongside os imports if any, or keep it as a synchronous require if there's a specific reason to avoid top-level imports.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/utils/src/core/storage/runtime.ts` around lines 43 - 46, Replace the
CommonJS require in fileExists with an ESM import: move to a top-level ESM
import like `import { existsSync } from "node:fs";` and have fileExists continue
to return existsSync(path); (alternatively, if you prefer dynamic import, make
fileExists async and use `const { existsSync } = await import("node:fs");`);
update the function signature accordingly if you choose the async approach.
| constructor(toolName: string, logger: StorageLogger = noopLogger) { | ||
| this.toolName = toolName; | ||
| this.baseDir = join(homedir(), ".genesis-tools", toolName); | ||
| this.cacheDir = join(this.baseDir, "cache"); | ||
| this.configPath = join(this.baseDir, "config.json"); |
There was a problem hiding this comment.
Constrain toolName and cache file paths to the storage root.
Both values are joined directly into filesystem paths. new Storage("../other") or putCacheFile("../../.ssh/config", ...) can escape ~/.genesis-tools and touch arbitrary files on disk.
Suggested fix
-import { dirname, join } from "node:path";
+import { dirname, join, resolve, sep } from "node:path";
@@
constructor(toolName: string, logger: StorageLogger = noopLogger) {
+ const storageRoot = resolve(homedir(), ".genesis-tools");
+ const baseDir = resolve(storageRoot, toolName);
+
+ if (baseDir === storageRoot || !baseDir.startsWith(`${storageRoot}${sep}`)) {
+ throw new Error(`Invalid tool name: ${toolName}`);
+ }
+
this.toolName = toolName;
- this.baseDir = join(homedir(), ".genesis-tools", toolName);
+ this.baseDir = baseDir;
this.cacheDir = join(this.baseDir, "cache");
this.configPath = join(this.baseDir, "config.json");
this.logger = logger;
@@
private getCacheFilePath(relativePath: string): string {
- return join(this.cacheDir, relativePath);
+ const cacheRoot = resolve(this.cacheDir);
+ const filePath = resolve(cacheRoot, relativePath);
+
+ if (filePath === cacheRoot || !filePath.startsWith(`${cacheRoot}${sep}`)) {
+ throw new Error(`Invalid cache path: ${relativePath}`);
+ }
+
+ return filePath;
}Also applies to: 219-220
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/utils/src/core/storage/storage.ts` around lines 39 - 43, The
constructor and cache file methods currently join raw inputs into filesystem
paths (constructor: toolName -> baseDir/cacheDir/configPath; methods at/around
putCacheFile and getCachePath) allowing path traversal; fix by sanitizing
toolName (e.g., restrict to a safe token: use path.basename or validate with a
regex like /^[A-Za-z0-9_-]+$/ and throw on invalid/empty names) and by
resolving+validating any file path created under baseDir (use
path.resolve(join(baseDir, userPath)) then assert the resolved path
startsWith(baseDir) before using it, throwing an error if it escapes). Apply
this pattern in the constructor and in the cache-related functions referenced
(e.g., putCacheFile, getCachePath) so no user input can break out of
~/.genesis-tools/<toolName>.
| for (let i = 0; i < keys.length - 1; i++) { | ||
| const k = keys[i]; | ||
| if (!(k in current) || typeof current[k] !== "object") { | ||
| current[k] = {}; | ||
| } | ||
| current = current[k] as Record<string, unknown>; |
There was a problem hiding this comment.
Treat null and arrays as terminal values while walking config paths.
typeof null === "object", so an existing value like { oauth2: null } slips through this guard and the next assignment crashes. Arrays are also treated as mergeable objects here.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/utils/src/core/storage/storage.ts` around lines 148 - 153, When
walking the config path in the loop that iterates over keys (variables: keys,
current, k) treat null and arrays as terminal values so they get overwritten
with an object; update the guard so it not only checks typeof current[k] !==
"object" but also treats current[k] === null and Array.isArray(current[k]) as
reasons to replace the value. In other words, in the loop around keys/current/k,
change the condition that decides to set current[k] = {} to include null and
arrays (use a check like current[k] === null || Array.isArray(current[k]) or
combine with the existing typeof check) so existing null or array values don’t
leak through and crash on the next assignment.
| // Quick verification test for imports | ||
|
|
||
| import { formatTokens } from "./dist/core/formatting.js"; | ||
| import { Storage as StorageSubpath } from "./dist/core/storage/index.js"; | ||
| import { createLogger, debounce, formatDuration, Storage } from "./dist/index.js"; | ||
|
|
||
| console.log("Testing imports..."); | ||
|
|
||
| // Test Storage | ||
| const storage = new Storage("test-tool"); | ||
| console.log("✓ Storage imported:", storage.getBaseDir()); | ||
|
|
||
| // Test formatting | ||
| console.log("✓ formatDuration:", formatDuration(125000)); | ||
| console.log("✓ formatTokens:", formatTokens(15000)); | ||
|
|
||
| // Test debounce | ||
| const debouncedFn = debounce(() => console.log("debounced"), 100); | ||
| console.log("✓ debounce works:", typeof debouncedFn); | ||
|
|
||
| // Test logger | ||
| const logger = createLogger({ level: "info" }); | ||
| console.log("✓ createLogger works:", logger.level); | ||
|
|
||
| // Test subpath import | ||
| const storageSubpath = new StorageSubpath("test-subpath"); | ||
| console.log("✓ Subpath import works:", storageSubpath.getCacheDir()); | ||
|
|
||
| console.log("\nAll imports verified successfully!"); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Make the smoke test fail on wrong behavior, not just missing imports.
Right now this only logs values, so regressions in helpers like formatDuration() or formatTokens() still end with “verified successfully”. A couple of assertions would turn it into an actionable post-build check.
Suggested fix
+import { strict as assert } from "node:assert";
+
// Quick verification test for imports
import { formatTokens } from "./dist/core/formatting.js";
import { Storage as StorageSubpath } from "./dist/core/storage/index.js";
import { createLogger, debounce, formatDuration, Storage } from "./dist/index.js";
@@
// Test Storage
const storage = new Storage("test-tool");
+assert.ok(storage.getBaseDir().length > 0);
console.log("✓ Storage imported:", storage.getBaseDir());
@@
// Test formatting
-console.log("✓ formatDuration:", formatDuration(125000));
-console.log("✓ formatTokens:", formatTokens(15000));
+assert.equal(formatDuration(125000), "2m 5s");
+assert.equal(formatTokens(15000), "15.0K");
+console.log("✓ formatting helpers imported and behave as expected");
@@
// Test debounce
const debouncedFn = debounce(() => console.log("debounced"), 100);
+assert.equal(typeof debouncedFn, "function");
console.log("✓ debounce works:", typeof debouncedFn);
@@
// Test logger
const logger = createLogger({ level: "info" });
+assert.equal(logger.level, "info");
console.log("✓ createLogger works:", logger.level);
@@
// Test subpath import
const storageSubpath = new StorageSubpath("test-subpath");
+assert.ok(storageSubpath.getCacheDir().length > 0);
console.log("✓ Subpath import works:", storageSubpath.getCacheDir());📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Quick verification test for imports | |
| import { formatTokens } from "./dist/core/formatting.js"; | |
| import { Storage as StorageSubpath } from "./dist/core/storage/index.js"; | |
| import { createLogger, debounce, formatDuration, Storage } from "./dist/index.js"; | |
| console.log("Testing imports..."); | |
| // Test Storage | |
| const storage = new Storage("test-tool"); | |
| console.log("✓ Storage imported:", storage.getBaseDir()); | |
| // Test formatting | |
| console.log("✓ formatDuration:", formatDuration(125000)); | |
| console.log("✓ formatTokens:", formatTokens(15000)); | |
| // Test debounce | |
| const debouncedFn = debounce(() => console.log("debounced"), 100); | |
| console.log("✓ debounce works:", typeof debouncedFn); | |
| // Test logger | |
| const logger = createLogger({ level: "info" }); | |
| console.log("✓ createLogger works:", logger.level); | |
| // Test subpath import | |
| const storageSubpath = new StorageSubpath("test-subpath"); | |
| console.log("✓ Subpath import works:", storageSubpath.getCacheDir()); | |
| console.log("\nAll imports verified successfully!"); | |
| import { strict as assert } from "node:assert"; | |
| // Quick verification test for imports | |
| import { formatTokens } from "./dist/core/formatting.js"; | |
| import { Storage as StorageSubpath } from "./dist/core/storage/index.js"; | |
| import { createLogger, debounce, formatDuration, Storage } from "./dist/index.js"; | |
| console.log("Testing imports..."); | |
| // Test Storage | |
| const storage = new Storage("test-tool"); | |
| assert.ok(storage.getBaseDir().length > 0); | |
| console.log("✓ Storage imported:", storage.getBaseDir()); | |
| // Test formatting | |
| assert.equal(formatDuration(125000), "2m 5s"); | |
| assert.equal(formatTokens(15000), "15.0K"); | |
| console.log("✓ formatting helpers imported and behave as expected"); | |
| // Test debounce | |
| const debouncedFn = debounce(() => console.log("debounced"), 100); | |
| assert.equal(typeof debouncedFn, "function"); | |
| console.log("✓ debounce works:", typeof debouncedFn); | |
| // Test logger | |
| const logger = createLogger({ level: "info" }); | |
| assert.equal(logger.level, "info"); | |
| console.log("✓ createLogger works:", logger.level); | |
| // Test subpath import | |
| const storageSubpath = new StorageSubpath("test-subpath"); | |
| assert.ok(storageSubpath.getCacheDir().length > 0); | |
| console.log("✓ Subpath import works:", storageSubpath.getCacheDir()); | |
| console.log("\nAll imports verified successfully!"); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/utils/test-import.ts` around lines 1 - 29, Replace the current
console-only smoke test with real assertions so failures break the build: call
functions like formatDuration(125000) and formatTokens(15000) and assert their
outputs match expected strings, assert Storage("test-tool").getBaseDir() and
StorageSubpath("test-subpath").getCacheDir() return non-empty strings (or
specific expected paths), assert debounce(...) returns a function and that
createLogger({level:"info"}).level === "info"; throw or exit non-zero on any
mismatch so the script fails on wrong behavior rather than only logging values.
Summary
packages/utilssrc/utils/storageandsrc/utils/diffto use the new package@genesis-tools/utilsTest plan
packages/utilsbuilds with tsupSummary by CodeRabbit
Release Notes
New Features
Chores