Skip to content

Comments

Add CLI improvements, modular commands, and code simplifications#10

Merged
TonyCasey merged 1 commit intomainfrom
refactor-setup.js
Nov 11, 2025
Merged

Add CLI improvements, modular commands, and code simplifications#10
TonyCasey merged 1 commit intomainfrom
refactor-setup.js

Conversation

@TonyCasey
Copy link
Owner

@TonyCasey TonyCasey commented Nov 11, 2025


Summary of Changes

  • Released version 2.0.0, marking a major iteration.
  • Simplified CLI commands (commit-todo, review) with stubs for future features.
  • Introduced new command handlers for commit-todo and review commands.
  • Added a centralized system for CLI parsing, configuration migration, and file utilities.
  • Enhanced development tools and templates in the .dev/ folder for automation.
  • Refactored code to use a provider system with the template method pattern.

Checklist

  • Documentation updated if necessary
  • Tests added/updated to cover changes
  • Reviewers tagged for code review

Additional Notes

  • The Codex-related functionality has been simplified to reduce complexity.
  • All major command alterations align with

Summary by CodeRabbit

Release Notes

  • New Features

    • Centralised command routing system for streamlined CLI experience
    • Multi-provider AI tool support: Claude Code, Cursor, and Gemini
    • Automated development workspace initialisation with rules management
    • Configuration migration and management utilities for existing tool setups
  • Chores

    • Version bump to 2.0.0

- Bumped version to `2.0.0` to reflect a major iteration.
- Simplified `commit-todo-command.js`, replacing unused parameters with stubs for future implementation.
- Removed unused backup file `bin/setup.js.backup`.

refactor(commands): simplify and stub `commit-todo` and `review` commands

- Removed Codex-related logic from `commit-todo-command.js` and `review-command.js` to reduce complexity.
- Added placeholders for future implementation of `todo-commit` enforcement workflows.
- Updated `listMarkdownFiles` in `file-utils.js` to accept a `baseDir` parameter for improved versatility.
- Adjusted imports across modules to reflect the changes.

feat(commands): implement `commit-todo` and `review` command handlers

- Added `commit-todo` command for enforcing TODO commit policies.
- Introduced `review` command for analyzing codebase compliance with architecture and rules.
- Modularized commands in `lib/commands` for centralized management.
- Enhanced `.dev/` folder setup with templates and automation in `dev-workspace.js`.
- Enabled Codex manifest and context index generation across commands.

feat(cli): add CLI parser, migration handler, file utilities, and user prompts

- Introduced `cli-parser.js` to handle command and option parsing.
- Added `config-migrator.js` for configuration migration tasks.
- Developed `file-utils.js` for common file system operations with reusable utilities.
- Created `prompts.js` module for user interaction during configuration and setup.
- Included comprehensive tests for the `cli-parser` module, utilizing AAA principles.

refactor: create provider system with template method pattern (Phase 2)
@coderabbitai
Copy link

coderabbitai bot commented Nov 11, 2025

Walkthrough

This pull request introduces a major refactoring of the CLI infrastructure, extracting monolithic command handling into a modular, provider-based architecture. The changes include a new centralized CLI parser, command routing system, reusable command handlers, a provider factory pattern for AI tool setup (Claude, Cursor, Gemini), and comprehensive utilities for file management, templating, user prompts, and workspace configuration. Version bumped to 2.00.0.

Changes

Cohort / File(s) Summary
CLI & Argument Parsing
lib/cli-parser.js, __tests__/lib/cli-parser.test.js
New CLI parser module exporting parseArguments() and getCommandType() functions with comprehensive test coverage for flag handling, command extraction, and normalization.
Entry Point Refactor
bin/setup.js
Replaced ad-hoc command branching with centralized router using CLI parser; routes to command handlers via executeSetup, executeUpdate, executeReview, executeCommitTodo; added async IIFE for centralised error handling.
Command Handlers
lib/commands/setup-command.js, lib/commands/update-command.js, lib/commands/review-command.js, lib/commands/commit-todo-command.js, lib/commands/index.js
Five new command modules implementing discrete workflows: setup orchestrates language detection and tool configuration; update delegates to setup; review runs code analysis; commit-todo provides stub enforcement; index re-exports all handlers.
Provider System
lib/providers/base-provider.js, lib/providers/claude-provider.js, lib/providers/cursor-provider.js, lib/providers/gemini-provider.js, lib/providers/index.js
New provider architecture with abstract BaseProvider template-method class supporting hooks/commands/settings/additional setup flows; concrete providers for Claude, Cursor, and Gemini with provider factory.
File & Path Utilities
lib/file-utils.js
New dependency-injected file utilities: copyDirectorySync, copyPath, ensureDirectory, writeFile, listMarkdownFiles, makeExecutable.
User Interaction
lib/prompts.js
New module exposing interactive prompts via inquirer: language confirmation/selection, tool selection, migration actions, replace/overwrite confirmations with DI support.
Template & Manifest Management
lib/template-manager.js
New template discovery, copying, and generation utilities; functions to discover rule files, generate README blocks, context indices, and Codex manifests.
Workspace & Dev Setup
lib/dev-workspace.js
New module orchestrating .dev folder setup including architecture/feature/todo documentation, centralised rules, Codex manifest/guide, and AGENTS.md template.
Configuration Migration
lib/migration/config-migrator.js
New migration module: migrateConfigToLocal, migrateCursorConfig, removeExistingConfig for handling existing tool configurations during setup transitions.
Project Configuration
package.json, tsconfig.json.bak
Version bumped to 2.00.0; TypeScript backup config file deleted.

Sequence Diagram(s)

sequenceDiagram
    actor User
    participant bin/setup.js as CLI Entry
    participant cli-parser.js as Parser
    participant Command Handler
    participant Provider/Utility

    User->>CLI Entry: npm run setup [args]
    CLI Entry->>Parser: parseArguments(argv)
    Parser-->>CLI Entry: { command, flags, options }
    CLI Entry->>Parser: getCommandType(command)
    Parser-->>CLI Entry: normalised commandType
    
    alt commandType === 'setup'
        CLI Entry->>Command Handler: executeSetup(projectRoot, options)
        activate Command Handler
        Command Handler->>Provider/Utility: detectAndConfirmLanguage()
        Provider/Utility-->>Command Handler: language
        Command Handler->>Provider/Utility: selectAITools()
        Provider/Utility-->>Command Handler: tools[]
        loop for each tool
            Command Handler->>Provider/Utility: setupTool(toolName, ...)
            Provider/Utility->>Provider/Utility: provider.setup()
            note over Provider/Utility: Execute hooks, commands,<br/>settings, additional
            Provider/Utility-->>Command Handler: { success }
        end
        Command Handler->>Provider/Utility: setupDevFolder(...)
        Command Handler->>Provider/Utility: setupCentralizedRules(...)
        Command Handler->>Provider/Utility: writeCodexManifestAndIndex(...)
        Command Handler->>Provider/Utility: setupCodexGuide(...)
        Command Handler-->>CLI Entry: (success)
        deactivate Command Handler
    else commandType === 'update'
        CLI Entry->>Command Handler: executeUpdate(projectRoot, options)
        Command Handler->>Command Handler: delegates to executeSetup
    else commandType === 'review'
        CLI Entry->>Command Handler: executeReview(projectRoot, options)
        Command Handler->>Provider/Utility: refresh Codex guide
        Command Handler->>Provider/Utility: run CodeReviewer.analyze()
        Command Handler-->>CLI Entry: exit(1 if violations)
    else commandType === 'unknown'
        CLI Entry->>CLI Entry: log error, exit(1)
    end
    
    CLI Entry-->>User: process exit
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60–90 minutes

Areas requiring extra attention:

  • lib/commands/setup-command.js — Dense orchestration logic across multiple domain concerns (language detection, tool selection, workspace setup, migration, Codex configuration); verify all sub-function calls align with documented flow.
  • lib/providers/base-provider.js — Abstract template-method pattern; review hook copying logic, migration branching, and interaction with subclasses (Claude, Cursor, Gemini).
  • lib/template-manager.js — Complex manifest and index generation; verify rule discovery logic handles all language/path combinations correctly; check generated content structure.
  • lib/dev-workspace.js — Multiple sub-setup functions with file I/O and logging; ensure idempotent behaviour and error-resilient handling across edge cases.
  • lib/migration/config-migrator.js — File lifecycle management (copy, append, delete); verify action-specific messages and log states match expectations.
  • bin/setup.js → lib/cli-parser.js integration — New control flow; ensure all parsed options and commands are correctly routed and consumed by handlers.

Poem

🐰 A rabbit hops through code so new,
Commands parsed, providers too!
Providers setup Claude and friends,
From start to finish—code transcends!
Two-point-oh brings order clear,
Our refactored year of cheer! 🎉

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and accurately captures the main refactoring: CLI improvements via a centralized parser, modular command architecture (setup-command, review-command, update-command, commit-todo-command), and code simplifications through new abstractions (BaseProvider, file-utils, config-migrator, template-manager).
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch refactor-setup.js

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@TonyCasey TonyCasey merged commit 733e17e into main Nov 11, 2025
2 of 3 checks passed
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Nov 11, 2025

Greptile Overview

Greptile Summary

This PR refactors the CLI from a monolithic 1700+ line file into a modular architecture with separated concerns, introducing command handlers, a provider system using the Template Method pattern, and comprehensive utility modules.

Major Changes:

  • Extracted CLI parsing logic into dedicated cli-parser.js module with pure functions
  • Created modular command handlers (setup-command.js, update-command.js, review-command.js, commit-todo-command.js)
  • Implemented provider system with BaseProvider abstract class and concrete implementations for Claude, Gemini, and Cursor
  • Added utility modules for file operations (file-utils.js), user prompts (prompts.js), and template management (template-manager.js)
  • Introduced config migration system for handling existing configurations
  • Added comprehensive test coverage for CLI parser with 185 lines of tests following AAA pattern

Code Quality:

  • Follows Clean Architecture and Repository Pattern as specified in AGENTS.md:72
  • Uses dependency injection for testability (fs module, inquirer)
  • Pure functions with no side effects in library code
  • Proper separation: bin/ for entry point, lib/ for pure modules

Issues Found:

  • Version number format in package.json is non-standard (2.00.0 should be 2.0.0)
  • Provider factory doesn't handle kilo and roo tools that are selectable in the UI, will throw error at runtime if selected

Confidence Score: 3/5

  • This PR is moderately safe to merge with one critical runtime bug that needs fixing
  • Score reflects well-structured refactoring with strong adherence to SOLID principles and comprehensive testing, but includes one critical bug where selecting kilo or roo providers will cause runtime crashes. The version number issue is minor but should be corrected before release. The architecture improvements are significant and the code quality is high, following the repository's guidelines.
  • lib/providers/index.js needs immediate attention to handle missing provider implementations, and package.json should use proper semantic versioning format

Sequence Diagram

sequenceDiagram
    participant User
    participant CLI as bin/setup.js
    participant Parser as cli-parser
    participant SetupCmd as setup-command
    participant Prompts as prompts
    participant ProviderFactory as providers/index
    participant Provider as BaseProvider/Subclass
    participant DevWorkspace as dev-workspace
    participant TemplateManager as template-manager

    User->>CLI: Execute ai-dotfiles-manager
    CLI->>Parser: parseArguments(argv)
    Parser-->>CLI: {command, options}
    CLI->>Parser: getCommandType(command)
    Parser-->>CLI: 'setup'

    CLI->>SetupCmd: executeSetup(projectRoot, options)
    
    SetupCmd->>SetupCmd: detectAndConfirmLanguage()
    alt autoYes mode
        SetupCmd->>SetupCmd: Use detected/default language
    else interactive mode
        SetupCmd->>Prompts: confirmLanguage(detectedLanguage)
        Prompts-->>SetupCmd: confirmed language
    end

    SetupCmd->>SetupCmd: selectAITools(autoYes)
    alt autoYes mode
        SetupCmd->>SetupCmd: Default to ['claude']
    else interactive mode
        SetupCmd->>Prompts: selectTools()
        Prompts-->>SetupCmd: ['claude', 'gemini', ...]
    end

    loop For each selected tool
        SetupCmd->>ProviderFactory: setupTool(tool, projectRoot, templatesDir, options)
        ProviderFactory->>ProviderFactory: createProvider(tool, ...)
        ProviderFactory->>Provider: new Provider(...)
        Provider-->>ProviderFactory: provider instance
        ProviderFactory->>Provider: provider.setup()
        
        Provider->>Provider: Check for existing config
        alt Existing config found
            Provider->>Prompts: promptMigrationAction()
            Prompts-->>Provider: migration action
            Provider->>Provider: handleExistingConfig(action)
        end
        
        Provider->>Provider: setupHooks()
        Provider->>Provider: setupCommands()
        Provider->>Provider: setupSettings()
        Provider->>Provider: setupAdditional()
        Provider-->>ProviderFactory: {success: true}
        ProviderFactory-->>SetupCmd: setup result
    end

    SetupCmd->>DevWorkspace: setupDevFolder(projectRoot, language, false, {})
    DevWorkspace->>TemplateManager: generateArchitectureDoc()
    TemplateManager-->>DevWorkspace: architecture content
    DevWorkspace-->>SetupCmd: done

    SetupCmd->>DevWorkspace: setupCentralizedRules(projectRoot, language, ...)
    DevWorkspace-->>SetupCmd: done

    alt !noCodexGuide
        SetupCmd->>DevWorkspace: ensureAgentsTemplate(projectRoot)
        SetupCmd->>TemplateManager: discoverRuleFiles(projectRoot, language)
        TemplateManager-->>SetupCmd: discovered rules
        SetupCmd->>DevWorkspace: writeCodexManifestAndIndex(...)
        SetupCmd->>DevWorkspace: setupCodexGuide(...)
    end

    alt language === 'typescript'
        SetupCmd->>SetupCmd: setupTypeScriptConfig(projectRoot, false, templatesDir)
    end

    SetupCmd->>SetupCmd: printNextSteps(tools, language, false)
    SetupCmd-->>CLI: done
    CLI-->>User: Success message
Loading

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

20 files reviewed, 2 comments

Edit Code Review Agent Settings | Greptile

{
"name": "ai-dotfiles-manager",
"version": "1.14.0",
"version": "2.00.0",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

syntax: version format should be 2.0.0 not 2.00.0 (semantic versioning format)

Suggested change
"version": "2.00.0",
"version": "2.0.0",
Prompt To Fix With AI
This is a comment left during a code review.
Path: package.json
Line: 3:3

Comment:
**syntax:** version format should be `2.0.0` not `2.00.0` (semantic versioning format)

```suggestion
  "version": "2.0.0",
```

How can I resolve this? If you propose a fix, please make it concise.

Comment on lines +18 to +28
function createProvider(toolName, projectRoot, templatesDir, options = {}) {
switch (toolName) {
case 'claude':
return new ClaudeProvider(projectRoot, templatesDir, options);
case 'gemini':
return new GeminiProvider(projectRoot, templatesDir, options);
case 'cursor':
return new CursorProvider(projectRoot, templatesDir, options);
default:
throw new Error(`Unknown tool: ${toolName}`);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: createProvider will throw error for kilo and roo tools that are selectable in prompts (lib/prompts.js:65-66), causing runtime crash if user selects them

Prompt To Fix With AI
This is a comment left during a code review.
Path: lib/providers/index.js
Line: 18:28

Comment:
**logic:** `createProvider` will throw error for `kilo` and `roo` tools that are selectable in prompts (`lib/prompts.js:65-66`), causing runtime crash if user selects them

How can I resolve this? If you propose a fix, please make it concise.

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +53 to +76
async function selectTools(inquirerModule = inquirer) {
const { tools } = await inquirerModule.prompt([
{
type: 'checkbox',
name: 'tools',
message: 'Which AI tools would you like to configure? (Space to select)',
choices: [
{ name: '✨ Select All', value: 'all', checked: true },
new inquirerModule.Separator(),
{ name: 'Claude Code', value: 'claude', checked: true },
{ name: 'Gemini CLI', value: 'gemini', checked: true },
{ name: 'Cursor', value: 'cursor', checked: false },
{ name: 'Kilo Code', value: 'kilo', checked: false },
{ name: 'Roo Code', value: 'roo', checked: false },
new inquirerModule.Separator(),
{ name: '🚫 None (Codex only — skip provider folders)', value: 'none', checked: false },
],
},
]);

// Handle "Select All" option
if (tools.includes('all')) {
return ['claude', 'gemini', 'cursor', 'kilo', 'roo'];
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Avoid offering unsupported Kilo/Roo providers

The tool-selection prompt advertises Kilo Code and Roo Code and returns them whenever the default “Select All” option remains checked, but the provider factory only knows about claude, gemini, and cursor. Choosing the defaults (or explicitly picking Kilo/Roo) causes setupTool to throw Unknown tool: kilo/roo and abort the setup process before any configuration is applied. Users running ai-dotfiles-manager setup interactively will hit this failure path immediately. Either implement providers for these entries or remove them from the prompt until they are supported.

Useful? React with 👍 / 👎.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Nitpick comments (8)
lib/commands/review-command.js (1)

22-30: Log Codex refresh failures.
Silent failure hides actionable feedback when detectLanguage/discoverRuleFiles/setupCodexGuide throw. Emitting a warning (e.g., console.warn) keeps the command non-fatal while still surfacing the issue to the operator.

lib/providers/cursor-provider.js (2)

13-28: Consider extending BaseProvider for architectural consistency.

Unlike ClaudeProvider and GeminiProvider, CursorProvider doesn't extend BaseProvider. This creates architectural inconsistency and duplicates DI patterns (fs, log) that are already provided by the base class.

Consider refactoring to extend BaseProvider:

-class CursorProvider {
+class CursorProvider extends BaseProvider {
   constructor(projectRoot, templatesDir, options = {}) {
-    this.projectRoot = projectRoot;
-    this.templatesDir = templatesDir;
-    this.autoYes = options.autoYes || false;
-    this.fs = options.fs || fs;
-    this.log = options.log || console.log;
+    super('cursor', 'Cursor', projectRoot, templatesDir, options);
     
     this.cursorRulesPath = path.join(projectRoot, '.cursorrules');
-    this.templatePath = path.join(templatesDir, 'cursor', '.cursorrules');
+    this.templatePath = path.join(this.templateDir, '.cursorrules');
   }

56-62: Consider externalizing the local rules template content.

The .cursorrules.local content is hardcoded. For consistency with the template-driven approach used elsewhere, consider storing this content in a template file.

lib/file-utils.js (1)

16-31: Consider adding error handling for robustness.

The function lacks error handling for file system operations (mkdirSync, readdirSync, statSync, copyFileSync). Whilst copyPath wraps this in a try-catch, direct callers of this function would encounter unhandled exceptions.

Consider either:

  1. Adding internal error handling with informative error messages, or
  2. Documenting that this function may throw and should be wrapped by callers
lib/commands/setup-command.js (4)

12-13: Combine imports from the same module.

Lines 12-13 import from ../template-manager separately. Consider combining them into a single destructuring statement for better readability.

Apply this diff:

-const { discoverRuleFiles } = require('../template-manager');
-const { getPackageRoot, getTemplatesDir } = require('../template-manager');
+const { discoverRuleFiles, getPackageRoot, getTemplatesDir } = require('../template-manager');

23-73: Consider adding error handling.

The executeSetup function performs multiple async operations (file I/O, user prompts, tool setup) without try-catch error handling. If any operation fails, the error will propagate unhandled, potentially resulting in a poor user experience with cryptic error messages.

Consider wrapping the setup flow in a try-catch block:

 async function executeSetup(projectRoot, options = {}) {
   const autoYes = options.autoYes || false;
   const noCodexGuide = options.noCodexGuide || false;
 
   console.log(chalk.blue.bold('\n🤖 AI Dotfiles Manager Setup\n'));
 
   if (autoYes) {
     console.log(chalk.gray('Running in non-interactive mode (--yes flag)\n'));
   }
 
+  try {
   // Step 1: Detect and confirm language
   const language = await detectAndConfirmLanguage(projectRoot, autoYes);
   // ... rest of the steps ...
   console.log(chalk.green.bold('\n✅ Setup complete!\n'));
   printNextSteps(tools, language, false);
+  } catch (error) {
+    console.error(chalk.red(`\n❌ Setup failed: ${error.message}\n`));
+    throw error;
+  }
 }

141-142: Move require statements to the top of the module.

The fs and path modules are required inside the function rather than at the module level. This is inconsistent with the rest of the file and considered a code smell.

Apply this diff:

 const chalk = require('chalk');
+const fs = require('fs');
+const path = require('path');
 const { detectLanguage } = require('../language-detector');
 // ... rest of imports
 
 async function setupTypeScriptConfig(projectRoot, isUpdate, templatesDir) {
-  const fs = require('fs');
-  const path = require('path');
-  
   console.log(chalk.blue('\n📦 Setting up TypeScript configuration files...'));

153-178: Add error handling for file operations.

The fs.copyFileSync operations at lines 172 and 176 can throw errors (e.g., permission denied, disk full, source not readable), but there's no error handling. This could result in the setup process failing midway through with an unhelpful error message.

Consider wrapping file operations in try-catch blocks:

 for (const { source, dest } of tsConfigFiles) {
   const sourcePath = path.join(templatesDir, 'languages', 'typescript', source);
   const destPath = path.join(projectRoot, dest);
 
   if (!fs.existsSync(sourcePath)) {
     console.log(chalk.yellow(`  ⚠ Template file ${source} not found, skipping`));
     continue;
   }
 
+  try {
     if (fs.existsSync(destPath)) {
       const backupPath = `${destPath}.bak`;
       let backupNumber = 1;
       let finalBackupPath = backupPath;
 
       while (fs.existsSync(finalBackupPath)) {
         finalBackupPath = `${destPath}.bak${backupNumber}`;
         backupNumber++;
       }
 
       fs.copyFileSync(destPath, finalBackupPath);
       console.log(chalk.yellow(`  ⚠ Backed up existing ${dest} to ${path.basename(finalBackupPath)}`));
     }
 
     fs.copyFileSync(sourcePath, destPath);
     console.log(chalk.green(`  ✓ Created ${dest}`));
+  } catch (error) {
+    console.log(chalk.red(`  ✗ Failed to copy ${dest}: ${error.message}`));
+  }
 }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting

📥 Commits

Reviewing files that changed from the base of the PR and between 78707d3 and 6b11886.

📒 Files selected for processing (20)
  • __tests__/lib/cli-parser.test.js (1 hunks)
  • bin/setup.js (2 hunks)
  • lib/cli-parser.js (1 hunks)
  • lib/commands/commit-todo-command.js (1 hunks)
  • lib/commands/index.js (1 hunks)
  • lib/commands/review-command.js (1 hunks)
  • lib/commands/setup-command.js (1 hunks)
  • lib/commands/update-command.js (1 hunks)
  • lib/dev-workspace.js (1 hunks)
  • lib/file-utils.js (1 hunks)
  • lib/migration/config-migrator.js (1 hunks)
  • lib/prompts.js (1 hunks)
  • lib/providers/base-provider.js (1 hunks)
  • lib/providers/claude-provider.js (1 hunks)
  • lib/providers/cursor-provider.js (1 hunks)
  • lib/providers/gemini-provider.js (1 hunks)
  • lib/providers/index.js (1 hunks)
  • lib/template-manager.js (1 hunks)
  • package.json (1 hunks)
  • tsconfig.json.bak (0 hunks)
💤 Files with no reviewable changes (1)
  • tsconfig.json.bak
🧰 Additional context used
🪛 Biome (2.1.2)
bin/setup.js

[error] 60-60: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Safe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)

🔇 Additional comments (25)
package.json (1)

3-3: Version bump looks good.
No concerns with the release metadata update.

lib/commands/commit-todo-command.js (1)

17-32: Stub communicates status clearly.
Graceful messaging for the forthcoming todo enforcement keeps users informed without breaking flow.

lib/commands/update-command.js (1)

15-18: Delegation to setup is appropriate.
Reusing executeSetup keeps the update path consistent with the primary install flow.

__tests__/lib/cli-parser.test.js (1)

8-184: Comprehensive parser coverage.
Thorough assertions over flags, aliases, and fallback behaviour give strong confidence in the new CLI parser.

lib/providers/index.js (2)

18-29: LGTM! Clean factory pattern implementation.

The factory function correctly instantiates providers based on tool name and handles unknown tools appropriately.


39-42: LGTM! Simple and effective setup orchestration.

The function correctly delegates to the provider's setup method. Error handling is appropriately left to the caller.

lib/commands/index.js (1)

1-16: LGTM! Clean module organization.

The centralized command handler exports provide a clean API surface for the CLI entry point.

lib/cli-parser.js (1)

39-53: LGTM! Solid command normalization.

The function correctly handles command aliases and provides sensible defaults.

lib/providers/gemini-provider.js (2)

19-31: LGTM! Well-structured command setup.

The method properly validates template existence and provides clear user feedback.


36-42: LGTM! Clean settings setup.

The method correctly delegates to the base provider's template copying functionality.

lib/providers/cursor-provider.js (1)

71-100: LGTM! Robust existing configuration handling.

The method properly handles user choices and delegates migration tasks appropriately.

lib/providers/claude-provider.js (3)

22-52: LGTM! Efficient global command setup with content comparison.

The method intelligently avoids unnecessary file writes by comparing content before copying.


57-83: LGTM! Conservative and user-friendly settings management.

The method appropriately preserves existing project-specific settings in non-interactive mode whilst prompting for confirmation in interactive mode.


88-100: LGTM! Straightforward workflows setup.

The method correctly handles optional workflows directory with appropriate validation.

lib/file-utils.js (4)

78-82: LGTM! Simple directory creation utility.

The function correctly uses recursive: true to create parent directories as needed.


90-94: LGTM! Clean file writing with parent directory creation.

The function properly ensures parent directories exist before writing.


103-113: LGTM! Robust markdown file listing with proper error handling.

The function correctly handles missing directories and normalises paths to POSIX format.


120-126: LGTM! Cross-platform executable flag handling.

The function correctly handles Windows platforms where chmod may fail.

bin/setup.js (3)

14-30: LGTM! Clean argument parsing and flag handling.

The centralized CLI parser provides a clean separation of concerns and proper early exit behaviour for version and help flags.


72-79: LGTM! Comprehensive error handling.

The error handling properly logs both the error message and stack trace before exiting with a non-zero status code.


84-114: LGTM! Comprehensive and well-organised help documentation.

The help message provides clear usage information, examples, and proper formatting.

lib/commands/setup-command.js (4)

81-109: LGTM!

The language detection and confirmation logic is well-structured, handling both auto and interactive modes gracefully with appropriate defaults.


116-131: LGTM!

The tool selection logic is clear and handles both auto and interactive modes appropriately.


189-246: LGTM!

The printNextSteps function provides comprehensive, well-formatted guidance to users based on their configuration choices, enhancing the user experience significantly.


248-254: LGTM!

The module exports are appropriate and expose all the necessary functions for external use.

Comment on lines +59 to +64
case 'commit-todo':
const subCommand = parsed.commandArgs[0] || 'enforce';
await executeCommitTodo(PROJECT_ROOT, {
noCodexGuide: parsed.options.noCodexGuide,
}, subCommand);
break;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Fix variable declaration scope in switch statement.

The const subCommand declaration at line 60 is accessible from other switch cases, which violates block scoping expectations and creates a potential temporal dead zone (TDZ) issue. This is flagged by the static analysis tool.

Apply this diff to properly scope the declaration:

       case 'commit-todo':
+        {
         const subCommand = parsed.commandArgs[0] || 'enforce';
         await executeCommitTodo(PROJECT_ROOT, {
           noCodexGuide: parsed.options.noCodexGuide,
         }, subCommand);
         break;
+        }
📝 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.

Suggested change
case 'commit-todo':
const subCommand = parsed.commandArgs[0] || 'enforce';
await executeCommitTodo(PROJECT_ROOT, {
noCodexGuide: parsed.options.noCodexGuide,
}, subCommand);
break;
case 'commit-todo':
{
const subCommand = parsed.commandArgs[0] || 'enforce';
await executeCommitTodo(PROJECT_ROOT, {
noCodexGuide: parsed.options.noCodexGuide,
}, subCommand);
break;
}
🧰 Tools
🪛 Biome (2.1.2)

[error] 60-60: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.

The declaration is defined in this switch clause:

Safe fix: Wrap the declaration in a block.

(lint/correctness/noSwitchDeclarations)

🤖 Prompt for AI Agents
In bin/setup.js around lines 59 to 64, the const subCommand is declared directly
in the switch clause and can leak into other cases causing TDZ/static analysis
issues; fix by introducing a block scope for the 'commit-todo' case (add { }
around the case body) and move the const subCommand declaration and its await
executeCommitTodo call inside that block so the variable is block-scoped to this
case only.

Comment on lines +12 to +32
function parseArguments(argv = []) {
const flags = argv.filter(arg => arg.startsWith('-'));
const positionals = argv.filter(arg => !arg.startsWith('-'));
const command = positionals[0] || null;
const commandIndex = typeof command === 'string' ? argv.indexOf(command) : -1;

return {
command,
commandArgs: commandIndex >= 0 ? argv.slice(commandIndex + 1) : [],
flags,
options: {
autoYes: flags.includes('--yes') || flags.includes('-y'),
noCodexGuide: flags.includes('--no-codex-guide'),
detailed: flags.includes('--detailed'),
json: flags.includes('--json'),
fix: flags.includes('--fix'),
help: flags.includes('--help') || flags.includes('-h'),
version: flags.includes('--version') || flags.includes('-v'),
}
};
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Fix commandArgs calculation for edge case when command is null.

When no positional arguments are provided, commandIndex is -1, causing argv.slice(-1 + 1) to return the entire argv array (including flags) instead of an empty array.

Example:

parseArguments(['--yes', '--help'])
// command = null, commandIndex = -1
// commandArgs = argv.slice(0) = ['--yes', '--help'] ❌
// Expected: commandArgs = []

Apply this diff to fix the issue:

  return {
    command,
-   commandArgs: commandIndex >= 0 ? argv.slice(commandIndex + 1) : [],
+   commandArgs: command !== null ? argv.slice(commandIndex + 1) : [],
    flags,
    options: {
🤖 Prompt for AI Agents
In lib/cli-parser.js around lines 12 to 32, the commandArgs calculation uses
commandIndex when command is null which yields -1 and makes argv.slice(0) return
all flags; change the logic to only compute/slice when a command exists and was
found: determine commandIndex only if command is non-null and present in argv
(or simply branch on command truthiness) and set commandArgs to an empty array
when there is no command, otherwise use argv.slice(commandIndex + 1) to capture
only arguments after the command.

Comment on lines +41 to +71
function copyPath(source, target, replace = true, fsModule = fs) {
const isDirectory = fsModule.statSync(source).isDirectory();

if (fsModule.existsSync(target)) {
if (!replace) {
return { success: false, skipped: true };
}

const stats = fsModule.lstatSync(target);
if (stats.isDirectory()) {
fsModule.rmSync(target, { recursive: true });
} else {
fsModule.unlinkSync(target);
}
}

try {
if (isDirectory) {
copyDirectorySync(source, target, fsModule);
} else {
const parent = path.dirname(target);
if (!fsModule.existsSync(parent)) {
fsModule.mkdirSync(parent, { recursive: true });
}
fsModule.copyFileSync(source, target);
}
return { success: true };
} catch (error) {
return { success: false, error: error.message };
}
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Potential error: statSync outside try-catch block.

Line 42 calls statSync outside the try-catch block, which could throw an error if the source path doesn't exist. This would result in an unhandled exception rather than the expected { success: false, error } return value.

Apply this diff to fix:

 function copyPath(source, target, replace = true, fsModule = fs) {
+  try {
   const isDirectory = fsModule.statSync(source).isDirectory();

   if (fsModule.existsSync(target)) {
     // ... existing code ...
   }

-  try {
     if (isDirectory) {
       copyDirectorySync(source, target, fsModule);
     } else {
       // ... existing code ...
     }
     return { success: true };
   } catch (error) {
     return { success: false, error: error.message };
   }
 }

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In lib/file-utils.js around lines 41 to 71, the call to
fsModule.statSync(source) is made outside the try-catch so a missing/invalid
source throws an unhandled exception; wrap the statSync (and any dependent logic
like isDirectory determination and subsequent file checks) inside the existing
try-catch so errors from statSync are caught and returned as { success: false,
error: error.message }, or alternatively perform a safe exists check before
statSync to early-return an error — ensure all filesystem ops that can throw are
inside try-catch and preserve the original return shapes.

Comment on lines +54 to +84
const { tools } = await inquirerModule.prompt([
{
type: 'checkbox',
name: 'tools',
message: 'Which AI tools would you like to configure? (Space to select)',
choices: [
{ name: '✨ Select All', value: 'all', checked: true },
new inquirerModule.Separator(),
{ name: 'Claude Code', value: 'claude', checked: true },
{ name: 'Gemini CLI', value: 'gemini', checked: true },
{ name: 'Cursor', value: 'cursor', checked: false },
{ name: 'Kilo Code', value: 'kilo', checked: false },
{ name: 'Roo Code', value: 'roo', checked: false },
new inquirerModule.Separator(),
{ name: '🚫 None (Codex only — skip provider folders)', value: 'none', checked: false },
],
},
]);

// Handle "Select All" option
if (tools.includes('all')) {
return ['claude', 'gemini', 'cursor', 'kilo', 'roo'];
}

// Handle "None" option
if (tools.includes('none')) {
return [];
}

return tools;
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Stop auto-selecting every provider

With “Select All” pre-checked, the returned selection always contains every provider even when the user unticks some (e.g. Cursor stays unchecked in the UI but is still returned). This contradicts what the checkbox list communicates and prevents opting into a subset, so the CLI will set up unwanted providers. Please make “Select All” opt-in and strip the sentinel values before returning the list.

-        { name: '✨ Select All', value: 'all', checked: true },
+        { name: '✨ Select All', value: 'all', checked: false },
@@
-  // Handle "Select All" option
-  if (tools.includes('all')) {
-    return ['claude', 'gemini', 'cursor', 'kilo', 'roo'];
-  }
-  
-  // Handle "None" option
-  if (tools.includes('none')) {
-    return [];
-  }
-  
-  return tools;
+  // Handle "None" option
+  if (tools.includes('none')) {
+    return [];
+  }
+
+  // Handle "Select All" option
+  if (tools.includes('all')) {
+    return ['claude', 'gemini', 'cursor', 'kilo', 'roo'];
+  }
+
+  return tools.filter(tool => tool !== 'all');

@TonyCasey TonyCasey deleted the refactor-setup.js branch November 11, 2025 23:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant