Refactor src/services into single-responsibility methods and split orchestration flows#8
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors several src/services/* modules by decomposing large flows into smaller single-responsibility helpers, and updates the Git clone path to avoid embedding tokens in clone URLs (moving to header-based auth).
Changes:
- Split orchestration/setup flows into focused helper methods across review, AI setup, GitLab setup, and repo resolution services.
- Extracted formatting helpers for MR markdown generation and GitLab API route builders to reduce duplication.
- Adjusted clone implementation to use
gitwith auth headers and added token masking in clone error messages.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/services/review-orchestrator.service.ts | Refactors context/diff preparation and AI execution into helpers. |
| src/services/mr-markdown-formatter.service.ts | Extracts commit/diff/stats formatting helpers from markdown assembly. |
| src/services/gitlab-setup.service.ts | Splits GitLab setup wizard into intro, URL selection, token help, and validation loop helpers. |
| src/services/gitlab-instance-manager.service.ts | Extracts choice-building and default-marking helpers for instance switching. |
| src/services/gitlab-data-provider.service.ts | Separates URL parsing, config lookup, fetch+summary rendering, and error handling. |
| src/services/gitlab-api.service.ts | Extracts MR route builders and endpoint-specific fetch helpers. |
| src/services/git-repo-resolver.service.ts | Splits local repo detection, download prompt, config lookup, and clone-with-feedback flow. |
| src/services/git-operations.service.ts | Changes clone implementation to use header-based auth + token masking; factors out path/env helpers. |
| src/services/context-manager.service.ts | Extracts directory/bootstrap, prompt copy, markdown discovery, and prompt assembly helpers. |
| src/services/ai-setup.service.ts | Splits AI tool setup UX into helper methods and centralizes tool validation steps. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| onClose: (code: number | null) => { | ||
| if (code === 0) { | ||
| Display.info('\n────────────────────────────────────────────────────────'); | ||
| Display.success('Analysis completed'); | ||
| resolve(); | ||
| } else { |
There was a problem hiding this comment.
reviewSpinner is only stopped when the AI process emits the first stdout/stderr chunk. If the tool exits successfully without producing output, the spinner will keep running and subsequent output will be printed while the spinner is active. Consider stopping/succeeding the spinner in the code === 0 branch when firstChunk is still true (or always stop it in onClose).
| private static buildCloneEnv(token: string): Record<string, string | undefined> { | ||
| const basicAuth = Buffer.from(`oauth2:${token}`).toString('base64'); | ||
| return { | ||
| ...process.env, | ||
| GIT_CONFIG_COUNT: '1', | ||
| GIT_CONFIG_KEY_0: 'http.extraHeader', | ||
| GIT_CONFIG_VALUE_0: `Authorization: Basic ${basicAuth}`, | ||
| }; | ||
| } | ||
|
|
||
| private static runClone(repoUrl: string, token: string, targetPath: string): void { | ||
| execFileSync('git', ['clone', '--depth', '1', repoUrl, targetPath], { | ||
| stdio: 'inherit', | ||
| env: GitOperationsService.buildCloneEnv(token), | ||
| }); | ||
| } | ||
|
|
There was a problem hiding this comment.
buildCloneEnv overwrites any pre-existing GIT_CONFIG_COUNT / GIT_CONFIG_KEY_* / GIT_CONFIG_VALUE_* environment variables inherited from the parent process, which can break users who already rely on those settings. Prefer passing config via git -c http.extraHeader=... clone ... (or increment/append to the existing GIT_CONFIG_COUNT entries) so you don’t clobber existing Git env-based config.
| private static buildCloneEnv(token: string): Record<string, string | undefined> { | |
| const basicAuth = Buffer.from(`oauth2:${token}`).toString('base64'); | |
| return { | |
| ...process.env, | |
| GIT_CONFIG_COUNT: '1', | |
| GIT_CONFIG_KEY_0: 'http.extraHeader', | |
| GIT_CONFIG_VALUE_0: `Authorization: Basic ${basicAuth}`, | |
| }; | |
| } | |
| private static runClone(repoUrl: string, token: string, targetPath: string): void { | |
| execFileSync('git', ['clone', '--depth', '1', repoUrl, targetPath], { | |
| stdio: 'inherit', | |
| env: GitOperationsService.buildCloneEnv(token), | |
| }); | |
| } | |
| private static runClone(repoUrl: string, token: string, targetPath: string): void { | |
| const basicAuth = Buffer.from(`oauth2:${token}`).toString('base64'); | |
| execFileSync( | |
| 'git', | |
| [ | |
| '-c', | |
| `http.extraHeader=Authorization: Basic ${basicAuth}`, | |
| 'clone', | |
| '--depth', | |
| '1', | |
| repoUrl, | |
| targetPath, | |
| ], | |
| { | |
| stdio: 'inherit', | |
| }, | |
| ); | |
| } |
| private static displayIntro(): void { | ||
| Display.section('📋 GITLAB CONFIGURATION'); | ||
| Display.info( | ||
| '💡 TIP: Use a nickname to Identify this GitLab config (e.g. "work", "gitlab-org").', |
There was a problem hiding this comment.
Minor grammar: "Use a nickname to Identify" should be "use a nickname to identify" (lowercase verb).
| '💡 TIP: Use a nickname to Identify this GitLab config (e.g. "work", "gitlab-org").', | |
| '💡 TIP: Use a nickname to identify this GitLab config (e.g. "work", "gitlab-org").', |
src/serviceshad several large methods mixing I/O, control flow, rendering, and error handling in the same block. This PR decomposes those paths into focused helpers to improve readability, local reasoning, and safer evolution without changing behavior.High-priority flow decomposition
ai-setup.service.ts: split setup UX, tool selection, installation validation, install guidance, and retry decision into dedicated methods.review-orchestrator.service.ts: separated context assembly, repo-context merge, MR markdown build, and AI process execution/stream handling.gitlab-setup.service.ts: split intro rendering, instance name input, GitLab type/url selection, token help, and token validation loop.Medium-priority service refactors
gitlab-data-provider.service.ts: separated MR URL parsing, GitLab instance lookup, fetch execution, MR summary display, and fetch error handling.git-repo-resolver.service.ts: split local-repo detection, download prompt, instance resolution, and clone-with-feedback path.git-operations.service.ts: split target path resolution, clone-exists check, clone execution, and error masking.mr-markdown-formatter.service.ts: extracted stats/commits/diffs formatting helpers from markdown assembly.context-manager.service.ts: extracted directory bootstrap, prompt file copy flow, markdown file discovery, content read, and final prompt composition.Lower-priority cleanup
gitlab-api.service.ts: extracted MR route builders and endpoint-specific fetch helpers to remove repeated inline paths.gitlab-instance-manager.service.ts: extracted default-selection choices and default-flag mapping; simplified spacing output.Security-oriented adjustment in clone path
git clone.