dynamically fetch the latest connector version#142
Conversation
…ge and download the newest jar file
There was a problem hiding this comment.
Pull Request Overview
This PR dynamically fetches the latest connector version from the GitHub releases API instead of using a hardcoded version. It introduces automatic version checking and downloads the newest jar file when needed.
Key changes:
- Replaces hardcoded connector version with dynamic GitHub API fetching
- Adds version comparison logic using semver to determine if updates are needed
- Improves error handling throughout the codebase with proper type checking
Reviewed Changes
Copilot reviewed 7 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/service/sandbox-client-manager.ts | Major refactor to dynamically fetch latest version from GitHub API and handle automatic updates |
| src/cli-main.ts | Updates CLI commands to use new dynamic download system and improves error handling |
| package.json | Adds semver and glob dependencies for version comparison and file pattern matching |
| src/utils/env-utils.ts | Improves error handling with proper type checking |
| src/utils/cli-utils.ts | Adds never return type to logAndExit function |
| src/service/sandbox-svc.ts | Replaces null with empty string and undefined for better type safety |
| src/service/sandbox-datastore.ts | Adds explicit type annotation to array |
Comments suppressed due to low confidence (1)
src/service/sandbox-client-manager.ts:32
- [nitpick] The variable name 'cachedGithubResponse' should follow camelCase convention and be named 'cachedGitHubResponse' (capital H in GitHub).
let cachedGithubResponse: any | null = null;
| function unzipClient(filePath:string) { | ||
| const CLIENT_INSTALL_PATH = filePath.replace("download/", '').replace("-default.zip", ""); |
There was a problem hiding this comment.
The string replacement logic is fragile and could fail if the file path structure changes. This assumes the path contains 'download/' which may not always be true. Consider using path.basename() and proper path manipulation instead.
| function unzipClient(filePath:string) { | |
| const CLIENT_INSTALL_PATH = filePath.replace("download/", '').replace("-default.zip", ""); | |
| function unzipClient(filePath: string) { | |
| const baseName = path.basename(filePath); | |
| const CLIENT_INSTALL_PATH = baseName.endsWith('-default.zip') | |
| ? baseName.slice(0, -'-default.zip'.length) | |
| : baseName; |
| cliUtils.logAndExit(1, `Failed to fetch sandbox-client release info from GitHub!\n | ||
| 'No matching ZIP asset found in latest release'`); |
There was a problem hiding this comment.
The error message contains inconsistent formatting with both \n and an actual newline, plus an unnecessary extra newline and incorrectly placed quotes around the error description.
| cliUtils.logAndExit(1, `Failed to fetch sandbox-client release info from GitHub!\n | |
| 'No matching ZIP asset found in latest release'`); | |
| cliUtils.logAndExit(1, `Failed to fetch sandbox-client release info from GitHub!\nNo matching ZIP asset found in latest release`); |
|
|
||
| async function fetchReleasesFromGitHub() { | ||
| if (cachedGithubResponse) return cachedGithubResponse; | ||
| const { data } = await axios.get(GITHUB_API_URL, { headers: { 'User-Agent': 'sandbox-cli' } }); |
There was a problem hiding this comment.
This function makes a duplicate API call to GitHub even though the same data was already fetched and cached in fetchReleasesFromGitHub(). Consider reusing the cached response.
| const loggingFilePath = path.join(loggingPath, 'sandbox-client.log'); | ||
| const configPath = path.join(getCurrentSandboxFolder(), 'config.json'); | ||
| const latestJar = (await findLatestJar())!; | ||
| const loggingConfigPath = path.join(latestJar.path, '..', '..', 'conf', 'logback.xml'); |
There was a problem hiding this comment.
Using relative path navigation with '..' is fragile and unclear. Consider using path.dirname() or constructing the path from known base directories for better maintainability.
| const loggingConfigPath = path.join(latestJar.path, '..', '..', 'conf', 'logback.xml'); | |
| const loggingConfigPath = path.join(path.dirname(path.dirname(latestJar.path)), 'conf', 'logback.xml'); |
Dynamically fetch the latest connector version from GitHub release page and download the newest jar file