Skip to content

dynamically fetch the latest connector version - #142

Merged
herzykj merged 6 commits into
masterfrom
feature/instal-newest-connector
Sep 26, 2025
Merged

dynamically fetch the latest connector version#142
herzykj merged 6 commits into
masterfrom
feature/instal-newest-connector

Conversation

@herzykj

@herzykj herzykj commented Aug 4, 2025

Copy link
Copy Markdown
Collaborator

Dynamically fetch the latest connector version from GitHub release page and download the newest jar file

@herzykj
herzykj requested review from amaski and Copilot August 4, 2025 12:48

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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;

Comment on lines +155 to +156
function unzipClient(filePath:string) {
const CLIENT_INSTALL_PATH = filePath.replace("download/", '').replace("-default.zip", "");

Copilot AI Aug 4, 2025

Copy link

Choose a reason for hiding this comment

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

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.

Suggested change
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;

Copilot uses AI. Check for mistakes.
Comment thread src/service/sandbox-client-manager.ts Outdated
Comment on lines +123 to +124
cliUtils.logAndExit(1, `Failed to fetch sandbox-client release info from GitHub!\n
'No matching ZIP asset found in latest release'`);

Copilot AI Aug 4, 2025

Copy link

Choose a reason for hiding this comment

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

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.

Suggested change
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`);

Copilot uses AI. Check for mistakes.

async function fetchReleasesFromGitHub() {
if (cachedGithubResponse) return cachedGithubResponse;
const { data } = await axios.get(GITHUB_API_URL, { headers: { 'User-Agent': 'sandbox-cli' } });

Copilot AI Aug 4, 2025

Copy link

Choose a reason for hiding this comment

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

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.

Copilot uses AI. Check for mistakes.
Comment thread src/service/sandbox-client-manager.ts Outdated
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');

Copilot AI Aug 4, 2025

Copy link

Choose a reason for hiding this comment

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

Using relative path navigation with '..' is fragile and unclear. Consider using path.dirname() or constructing the path from known base directories for better maintainability.

Suggested change
const loggingConfigPath = path.join(latestJar.path, '..', '..', 'conf', 'logback.xml');
const loggingConfigPath = path.join(path.dirname(path.dirname(latestJar.path)), 'conf', 'logback.xml');

Copilot uses AI. Check for mistakes.
@herzykj
herzykj merged commit d1467ac into master Sep 26, 2025
3 checks passed
@herzykj
herzykj deleted the feature/instal-newest-connector branch September 26, 2025 11:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants