Refactor CLI architecture to class-based modules and stricter responsibility boundaries#7
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors the CLI from function-export modules into class-based command/service/util layers to enforce clearer responsibility boundaries and a more consistent entrypoint pattern.
Changes:
- Migrates CLI commands (
init,validate,config,gitlab) to class-based entrypoints and updatesbin/moses.tsto delegate to these classes. - Consolidates services/utilities into cohesive static classes (e.g.,
GitlabService,RepositoryService,ConfigStore,Display,UrlParser). - Updates validate flow orchestration to use the new classes across config loading, MR fetching, diff limit checks, context building, and AI tool execution.
Reviewed changes
Copilot reviewed 26 out of 28 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/utils/url-parser.ts | Converts URL parsing/validation helpers into UrlParser static class. |
| src/utils/tool-validator.ts | Converts tool validation helpers into ToolValidator static class and adjusts version/help probing. |
| src/utils/display.ts | Wraps CLI output helpers into Display static class. |
| src/utils/config-store.ts | Centralizes config path/IO/permission utilities in ConfigStore. |
| src/services/repository.ts | Refactors repository detection/clone helpers into RepositoryService. |
| src/services/markdown.ts | Wraps MR markdown and diff counting into MarkdownService. |
| src/services/gitlab.ts | Wraps Axios client + retry + MR fetch into GitlabService. |
| src/services/context.ts | Wraps context dir management and prompt aggregation into ContextService. |
| src/services/context-scanner.ts | Wraps repo context scanning into ContextScannerService. |
| src/services/ai-tools.ts | Wraps AI tool prompt building + execution into AiReviewService. |
| src/commands/validate/review.ts | Updates validate review runner to use new services/utilities. |
| src/commands/validate/load-config.ts | Refactors config loading to ValidateConfigLoader using ConfigStore. |
| src/commands/validate/index.ts | Refactors validate orchestration into ValidateCommand.execute. |
| src/commands/validate/gitlab.ts | Refactors MR fetch flow into ValidateGitlabDataProvider. |
| src/commands/validate/check-limits.ts | Refactors diff limit check into DiffLimitChecker using MarkdownService. |
| src/commands/update-config/set-feedback-style.ts | Refactors feedback-style command into SetFeedbackStyleCommand. |
| src/commands/update-config/set-diff-limit.ts | Refactors diff-limit command into SetDiffLimitCommand. |
| src/commands/update-config/load-config.ts | Refactors config loader into UpdateConfigLoader. |
| src/commands/update-config/index.ts | Updates exports to expose the new command classes. |
| src/commands/init/index.ts | Refactors init flow into InitCommand. |
| src/commands/init/gitlab-wizard.ts | Refactors GitLab setup wizard into GitlabWizard. |
| src/commands/init/config-loader.ts | Refactors init config loader into InitConfigLoader. |
| src/commands/init/config-builder.ts | Refactors config builder/saver into InitConfigBuilder. |
| src/commands/init/ai-wizard.ts | Refactors AI tool setup wizard into AiWizard. |
| src/commands/gitlab/switch.ts | Refactors default GitLab switch command into SwitchGitlabCommand. |
| src/commands/gitlab/list.ts | Refactors GitLab listing command into ListGitlabsCommand. |
| package-lock.json | Adds an npm lockfile (in addition to existing pnpm lockfile). |
| bin/moses.ts | Updates CLI entrypoint wiring to call the new class-based commands. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const authUrl = repoUrl.replace(/^https:\/\//, `https://oauth2:${token}@`); | ||
|
|
||
| try { | ||
| execSync(`git clone --depth 1 ${authUrl} ${targetPath}`, { stdio: 'inherit' }); | ||
| return targetPath; | ||
| } catch (error) { | ||
| throw new Error( | ||
| `Failed to clone repository: ${error instanceof Error ? error.message : String(error)}`, | ||
| ); | ||
| try { | ||
| execSync(`git clone --depth 1 ${authUrl} ${targetPath}`, { stdio: 'inherit' }); | ||
| return targetPath; |
There was a problem hiding this comment.
The clone command embeds the raw token into the remote URL and executes via a shell-interpreted string. This can leak credentials (e.g., process list, git error output) and can break/inject if the token contains special characters. Prefer using a safer mechanism (e.g., GIT_ASKPASS/credential helper or passing args via spawn/execFile) and ensure the token is properly escaped/encoded.
| if (parts.length < 1) throw new Error('Invalid Merge Request URL'); | ||
|
|
There was a problem hiding this comment.
url.pathname.split('/-/') always returns at least one element, so parts.length < 1 will never be true. This means invalid MR URLs won’t be rejected and can produce an incorrect repo URL. Consider validating that the expected '/-/' segment is present (e.g., require parts.length >= 2, and/or also validate the remaining path contains 'merge_requests').
| if (parts.length < 1) throw new Error('Invalid Merge Request URL'); | |
| const mrPath = parts[1]; | |
| if (parts.length < 2 || !parts[0] || !mrPath?.startsWith('merge_requests/')) { | |
| throw new Error('Invalid Merge Request URL'); | |
| } | |
| const mrId = mrPath.slice('merge_requests/'.length); | |
| if (!mrId) throw new Error('Invalid Merge Request URL'); |
| Display.stream(chunk); | ||
| }, | ||
| onClose: (code: number | null) => { | ||
| if (code === 0) { |
There was a problem hiding this comment.
If the AI tool exits successfully without emitting any stdout/stderr, reviewSpinner is never stopped (it only stops on the first chunk). This can leave the spinner running even though the process completed. Consider stopping/succeeding the spinner in the code === 0 branch before resolving.
| if (code === 0) { | |
| if (code === 0) { | |
| stopSpinnerOnStart(); |
| Display.info( | ||
| ` Token: ${gitlab.token.replace(/./g, '*').substring(0, 10)}... (last 4 chars: ${gitlab.token.slice(-4)})`, | ||
| ); |
There was a problem hiding this comment.
This command prints the last 4 characters of the GitLab token. Even partial token disclosure is sensitive and can aid accidental leaks/screenshots. Consider never printing any token characters (e.g., show a fixed '********' placeholder or only token length / a non-reversible fingerprint).
| Display.info( | |
| ` Token: ${gitlab.token.replace(/./g, '*').substring(0, 10)}... (last 4 chars: ${gitlab.token.slice(-4)})`, | |
| ); | |
| Display.info(` Token: ********`); |
O projeto tinha módulos concentrando múltiplas responsabilidades e forte uso de exports baseados em funções, o que dificultava manutenção e evolução. Esta mudança reorganiza os fluxos principais para separar responsabilidades por módulo e padronizar o código em classes.
Arquitetura de comandos (orquestração explícita)
init,validate,gitlabeconfigforam migrados para classes de comando com ponto único de entrada (run/execute).Serviços e utilitários em classes coesas
GitlabService,AiReviewService,RepositoryService,ContextService,ContextScannerService,MarkdownService.Display,ConfigStore,UrlParser,ToolValidator.Separação de responsabilidades por camada
Exemplo do novo padrão