refactor: enhance environment checks and path resolution logic; improve reset command confirmation

Kinin-Code-Offical · Kinin-Code-Offical · commit e8eb12fc6d89 · 2025-12-21T05:19:23.000+03:00
diff --git a/README.md b/README.md
@@ -99,10 +99,24 @@ cloudsqlctl status
 
 Configuration and logs are stored in:
 
-- **User Scope (Default):** `%LOCALAPPDATA%\CloudSQLCTL`
-- **Machine Scope (Service):** `%ProgramData%\CloudSQLCTL`
+- **User Scope (Default)**:
+  - Config: `%LOCALAPPDATA%\CloudSQLCTL`
+  - Logs: `%LOCALAPPDATA%\CloudSQLCTL\logs`
+  - Binary: `%LOCALAPPDATA%\CloudSQLCTL\bin`
 
-You can view the exact paths used by your current environment:
+- **Machine Scope (Service)**:
+  - Config: `%ProgramData%\CloudSQLCTL`
+  - Logs: `%ProgramData%\CloudSQLCTL\logs`
+  - Binary: `%ProgramData%\CloudSQLCTL\bin`
+
+**Path Resolution Logic:**
+
+1. Environment Variables (`CLOUDSQLCTL_HOME`, etc.)
+2. Existing System Installation (`%ProgramData%`)
+3. Existing User Installation (`%LOCALAPPDATA%`)
+4. Default: User Scope (`%LOCALAPPDATA%`)
+
+Verify current paths with:
 
 ```powershell
 cloudsqlctl paths
@@ -115,11 +129,6 @@ This project uses GitHub Actions for automated releases.
 1. Tag a new version: `git tag v0.3.0`
 2. Push the tag: `git push origin v0.3.0`
 3. The workflow will build, test, sign (if configured), and publish artifacts to the [Releases](https://github.com/Kinin-Code-Offical/cloudsqlctl/releases) page.
-   `%LOCALAPPDATA%\CloudSQLCTL`
-
-- `config.json`: User preferences and selected instance.
-- `logs/`: Application and proxy logs.
-- `bin/`: The `cloud_sql_proxy.exe` binary.
 
 ## Troubleshooting
 
diff --git a/src/commands/check.ts b/src/commands/check.ts
@@ -1,18 +1,32 @@
 import { Command } from 'commander';
-import { checkEnvironment } from '../system/env.js';
+import { checkEnvironmentDetailed } from '../system/env.js';
 import { isServiceInstalled, isServiceRunning } from '../system/service.js';
 import { PATHS } from '../system/paths.js';
 import fs from 'fs-extra';
 import { logger } from '../core/logger.js';
+import { isAdmin } from '../system/powershell.js';
 
 export const checkCommand = new Command('check')
     .description('Verify full system configuration')
-    .action(async () => {
+    .option('--scope <scope>', 'Environment scope (User, Machine, or auto)', 'auto')
+    .action(async (options) => {
         try {
             logger.info('Checking system configuration...');
 
-            const envOk = await checkEnvironment('Machine');
-            logger.info(`Environment Variables (Machine): ${envOk ? 'OK' : 'MISSING/INCORRECT'}`);
+            let scope: 'Machine' | 'User' = 'User';
+            if (options.scope === 'auto') {
+                scope = await isAdmin() ? 'Machine' : 'User';
+            } else {
+                scope = options.scope;
+            }
+
+            const envCheck = await checkEnvironmentDetailed(scope);
+            if (envCheck.ok) {
+                logger.info(`Environment Variables (${scope}): OK`);
+            } else {
+                logger.warn(`Environment Variables (${scope}): ISSUES FOUND`);
+                envCheck.problems.forEach(p => logger.warn(`  - ${p}`));
+            }
 
             const binaryOk = await fs.pathExists(PATHS.PROXY_EXE);
             logger.info(`Proxy Binary: ${binaryOk ? 'OK' : 'MISSING'}`);
@@ -25,7 +39,7 @@ export const checkCommand = new Command('check')
                 logger.info(`Service Running: ${serviceRunning ? 'YES' : 'NO'}`);
             }
 
-            if (envOk && binaryOk && serviceInstalled) {
+            if (envCheck.ok && binaryOk) {
                 logger.info('System check passed.');
             } else {
                 logger.warn('System check failed. Run "cloudsqlctl repair" to fix.');
diff --git a/src/commands/doctor.ts b/src/commands/doctor.ts
@@ -1,7 +1,7 @@
 import { Command } from 'commander';
 import { logger } from '../core/logger.js';
 import { checkGcloudInstalled, getActiveAccount, checkAdc, listInstances } from '../core/gcloud.js';
-import { checkEnvironment } from '../system/env.js';
+import { checkEnvironmentDetailed } from '../system/env.js';
 import { isServiceInstalled } from '../system/service.js';
 import { getEnvVar } from '../system/powershell.js';
 import { PATHS, ENV_VARS } from '../system/paths.js';
@@ -46,13 +46,22 @@ export const doctorCommand = new Command('doctor')
         }
 
         // Check Environment Variables
-        const envOk = await checkEnvironment('Machine');
-        if (envOk) {
-            logger.info('✅ System Environment Variables are correct');
+        const envCheck = await checkEnvironmentDetailed('Machine');
+        if (envCheck.ok) {
+            logger.info('✅ Environment Variables (Machine) OK');
         } else {
-            logger.warn('⚠️ System Environment Variables are missing or incorrect');
+            // Try User scope
+            const userEnvCheck = await checkEnvironmentDetailed('User');
+            if (userEnvCheck.ok) {
+                logger.info('✅ Environment Variables (User) OK');
+            } else {
+                logger.warn('⚠️ Environment Variables issues found:');
+                envCheck.problems.forEach(p => logger.warn(`  [Machine] ${p}`));
+                userEnvCheck.problems.forEach(p => logger.warn(`  [User] ${p}`));
+            }
         }
 
+
         // Check Proxy Binary
         if (await fs.pathExists(PATHS.PROXY_EXE)) {
             logger.info('✅ Cloud SQL Proxy binary found');
diff --git a/src/commands/paths.ts b/src/commands/paths.ts
@@ -1,5 +1,5 @@
 import { Command } from 'commander';
-import { PATHS, ENV_VARS } from '../system/paths.js';
+import { PATHS, PATHS_SOURCE, PATHS_REASON } from '../system/paths.js';
 import chalk from 'chalk';
 
 export const pathsCommand = new Command('paths')
@@ -21,14 +21,8 @@ export const pathsCommand = new Command('paths')
         });
 
         console.log(chalk.bold('\nResolution Source:'));
-
-        if (process.env[ENV_VARS.PROXY_PATH]) {
-            console.log(`- ${chalk.green('Environment Variable')} (${ENV_VARS.PROXY_PATH}) is set.`);
-        } else if (PATHS.HOME.includes('ProgramData')) {
-            console.log(`- ${chalk.yellow('System Installation')} (ProgramData detected).`);
-        } else {
-            console.log(`- ${chalk.blue('User Installation')} (AppData detected).`);
-        }
+        console.log(`- Source: ${chalk.green(PATHS_SOURCE)}`);
+        console.log(`- Reason: ${PATHS_REASON}`);
 
         console.log('');
     });
diff --git a/src/commands/reset.ts b/src/commands/reset.ts
@@ -5,10 +5,18 @@ import fs from 'fs-extra';
 
 export const resetCommand = new Command('reset')
     .description('Reset configuration and remove local files')
-    .action(async () => {
+    .option('--yes', 'Confirm reset without prompting')
+    .action(async (options) => {
         try {
-            logger.warn('This will remove all configuration and logs.');
-            // In a real app, ask for confirmation
+            if (!options.yes) {
+                logger.warn('This will remove all configuration and logs.');
+                logger.warn(`  - Config: ${PATHS.CONFIG_FILE}`);
+                logger.warn(`  - Logs: ${PATHS.LOGS}`);
+                logger.error('Operation aborted. Use --yes to confirm.');
+                process.exit(1);
+            }
+
+            logger.info('Resetting configuration...');
             await fs.remove(PATHS.CONFIG_FILE);
             await fs.remove(PATHS.LOGS);
             // Maybe keep the binary?
diff --git a/src/core/updater.ts b/src/core/updater.ts
@@ -1,6 +1,7 @@
 import axios from 'axios';
 import fs from 'fs-extra';
 import crypto from 'crypto';
+import path from 'path';
 import { PATHS } from '../system/paths.js';
 import { logger } from './logger.js';
 
diff --git a/src/system/env.ts b/src/system/env.ts
@@ -1,6 +1,7 @@
 import { runPs, isAdmin } from './powershell.js';
 import { SYSTEM_PATHS, USER_PATHS, ENV_VARS } from './paths.js';
 import { logger } from '../core/logger.js';
+import fs from 'fs-extra';
 
 export async function setEnv(name: string, value: string, scope: 'Machine' | 'User') {
     if (scope === 'Machine' && !await isAdmin()) {
@@ -10,22 +11,68 @@ export async function setEnv(name: string, value: string, scope: 'Machine' | 'Us
     await runPs(`[Environment]::SetEnvironmentVariable("${name}", "${value}", "${scope}")`);
 }
 
-export async function setupEnvironment(scope: 'Machine' | 'User' = 'User') {
+export async function setupEnvironment(scope: 'Machine' | 'User' = 'User', force: boolean = false) {
     logger.info(`Configuring ${scope} environment variables...`);
     const paths = scope === 'Machine' ? SYSTEM_PATHS : USER_PATHS;
 
-    await setEnv(ENV_VARS.HOME, paths.HOME, scope);
-    await setEnv(ENV_VARS.LOGS, paths.LOGS, scope);
-    await setEnv(ENV_VARS.PROXY_PATH, paths.PROXY_EXE, scope);
+    const currentHome = await runPs(`[Environment]::GetEnvironmentVariable("${ENV_VARS.HOME}", "${scope}")`);
+    if (force || !currentHome) {
+        await setEnv(ENV_VARS.HOME, paths.HOME, scope);
+    }
+
+    const currentLogs = await runPs(`[Environment]::GetEnvironmentVariable("${ENV_VARS.LOGS}", "${scope}")`);
+    if (force || !currentLogs) {
+        await setEnv(ENV_VARS.LOGS, paths.LOGS, scope);
+    }
+
+    const currentProxy = await runPs(`[Environment]::GetEnvironmentVariable("${ENV_VARS.PROXY_PATH}", "${scope}")`);
+    if (force || !currentProxy) {
+        await setEnv(ENV_VARS.PROXY_PATH, paths.PROXY_EXE, scope);
+    }
 }
 
+/** @deprecated Use checkEnvironmentDetailed instead */
 export async function checkEnvironment(scope: 'Machine' | 'User' = 'User'): Promise<boolean> {
-    const paths = scope === 'Machine' ? SYSTEM_PATHS : USER_PATHS;
+    const result = await checkEnvironmentDetailed(scope);
+    return result.ok;
+}
+
+export async function checkEnvironmentDetailed(scope: 'Machine' | 'User' = 'User'): Promise<{ ok: boolean, problems: string[], values: { home: string, logs: string, proxy: string } }> {
     const home = await runPs(`[Environment]::GetEnvironmentVariable("${ENV_VARS.HOME}", "${scope}")`);
     const logs = await runPs(`[Environment]::GetEnvironmentVariable("${ENV_VARS.LOGS}", "${scope}")`);
     const proxy = await runPs(`[Environment]::GetEnvironmentVariable("${ENV_VARS.PROXY_PATH}", "${scope}")`);
 
-    return home === paths.HOME &&
-        logs === paths.LOGS &&
-        proxy === paths.PROXY_EXE;
+    const problems: string[] = [];
+
+    if (!home) {
+        problems.push(`${ENV_VARS.HOME} is not set.`);
+    } else {
+        try {
+            await fs.ensureDir(home);
+        } catch {
+            problems.push(`${ENV_VARS.HOME} path (${home}) does not exist and cannot be created.`);
+        }
+    }
+
+    if (!logs) {
+        problems.push(`${ENV_VARS.LOGS} is not set.`);
+    } else {
+        try {
+            await fs.ensureDir(logs);
+        } catch {
+            problems.push(`${ENV_VARS.LOGS} path (${logs}) does not exist and cannot be created.`);
+        }
+    }
+
+    if (!proxy) {
+        problems.push(`${ENV_VARS.PROXY_PATH} is not set.`);
+    } else if (!fs.existsSync(proxy)) {
+        problems.push(`${ENV_VARS.PROXY_PATH} points to non-existent file (${proxy}).`);
+    }
+
+    return {
+        ok: problems.length === 0,
+        problems,
+        values: { home, logs, proxy }
+    };
 }
diff --git a/src/system/paths.ts b/src/system/paths.ts
@@ -34,10 +34,15 @@ export const ENV_VARS = {
     GOOGLE_CREDS: 'GOOGLE_APPLICATION_CREDENTIALS'
 };
 
+export let PATHS_SOURCE: 'ENV' | 'SYSTEM_EXISTING' | 'USER_EXISTING' | 'DEFAULT_USER' = 'DEFAULT_USER';
+export let PATHS_REASON: string = 'Defaulting to User scope';
+
 function resolvePaths() {
     // 1. Priority: Environment Variables
     const envProxyPath = process.env[ENV_VARS.PROXY_PATH];
     if (envProxyPath) {
+        PATHS_SOURCE = 'ENV';
+        PATHS_REASON = `Environment variable ${ENV_VARS.PROXY_PATH} is set`;
         const proxyPath = envProxyPath;
         const home = process.env[ENV_VARS.HOME] || path.dirname(path.dirname(proxyPath)); // Guess home from proxy
         return {
@@ -56,6 +61,8 @@ function resolvePaths() {
 
     // 2. Fallback: Check for existing proxy file (System then User)
     if (fs.existsSync(SYSTEM_PATHS.PROXY_EXE)) {
+        PATHS_SOURCE = 'SYSTEM_EXISTING';
+        PATHS_REASON = `Found existing proxy at ${SYSTEM_PATHS.PROXY_EXE}`;
         return {
             ...SYSTEM_PATHS,
             CONFIG_DIR: SYSTEM_PATHS.HOME,
@@ -67,18 +74,15 @@ function resolvePaths() {
     }
 
     if (fs.existsSync(USER_PATHS.PROXY_EXE)) {
+        PATHS_SOURCE = 'USER_EXISTING';
+        PATHS_REASON = `Found existing proxy at ${USER_PATHS.PROXY_EXE}`;
         return USER_PATHS;
     }
 
-    // 3. Default: System Paths (Target for new installation)
-    return {
-        ...SYSTEM_PATHS,
-        CONFIG_DIR: SYSTEM_PATHS.HOME,
-        CONFIG_FILE: path.join(SYSTEM_PATHS.HOME, 'config.json'),
-        TEMP: path.join(SYSTEM_PATHS.HOME, 'temp'),
-        GCLOUD_DIR: path.join(SYSTEM_PATHS.HOME, 'gcloud'),
-        PID_FILE: path.join(SYSTEM_PATHS.HOME, 'proxy.pid'),
-    };
+    // 3. Default: User Paths (Target for new installation)
+    PATHS_SOURCE = 'DEFAULT_USER';
+    PATHS_REASON = 'Defaulting to User scope (no existing installation found)';
+    return USER_PATHS;
 }
 
 // Default to resolved paths
diff --git a/src/system/ps1.ts b/src/system/ps1.ts
@@ -11,26 +11,15 @@ const SCRIPTS = {
 Write-Host "Environment variables set."
 `,
     'install-proxy.ps1': `
-param([string]$ProxyPath = "${SYSTEM_PATHS.PROXY_EXE}")
-
-if (!(Test-Path $ProxyPath)) {
-    Write-Error "Proxy binary not found at $ProxyPath"
-    exit 1
-}
-
-$service = Get-Service -Name "${SERVICE_NAME}" -ErrorAction SilentlyContinue
-if ($service) {
-    Write-Host "Service ${SERVICE_NAME} already exists."
-} else {
-    New-Service -Name "${SERVICE_NAME}" -BinaryPathName $ProxyPath -StartupType Automatic
-    Write-Host "Service ${SERVICE_NAME} installed."
-}
+Write-Warning "DEPRECATED: Please use 'cloudsqlctl service install' instead."
+exit 1
 `,
     'uninstall-proxy.ps1': `
+Write-Warning "DEPRECATED: Please use 'cloudsqlctl service remove' instead."
 $service = Get-Service -Name "${SERVICE_NAME}" -ErrorAction SilentlyContinue
 if ($service) {
     Stop-Service -Name "${SERVICE_NAME}" -Force -ErrorAction SilentlyContinue
-    Remove-Service -Name "${SERVICE_NAME}" -ErrorAction SilentlyContinue
+    sc.exe delete "${SERVICE_NAME}"
     Write-Host "Service ${SERVICE_NAME} removed."
 }
 `,
@@ -49,3 +38,4 @@ export async function generateScripts() {
         logger.info(`Generated script: ${filePath}`);
     }
 }
+
diff --git a/src/system/selfheal.ts b/src/system/selfheal.ts
@@ -1,6 +1,6 @@
 import fs from 'fs-extra';
 import { PATHS } from './paths.js';
-import { checkEnvironment, setupEnvironment } from './env.js';
+import { checkEnvironmentDetailed, setupEnvironment } from './env.js';
 import { isServiceInstalled, installService } from './service.js';
 import { getLatestVersion, downloadProxy } from '../core/updater.js';
 import { generateScripts } from './ps1.js';
@@ -15,16 +15,19 @@ export async function selfHeal() {
     const scope = admin ? 'Machine' : 'User';
 
     // 1. Check Environment Variables
-    if (!await checkEnvironment(scope)) {
+    const envCheck = await checkEnvironmentDetailed(scope);
+    if (!envCheck.ok) {
         if (admin) {
             logger.warn(`Environment variables (${scope}) missing or incorrect. Fixing...`);
+            envCheck.problems.forEach(p => logger.debug(`  - ${p}`));
             try {
                 await setupEnvironment(scope);
             } catch (e) {
                 logger.error('Failed to fix environment variables', e);
             }
         } else {
             logger.warn('Environment variables missing or incorrect. Run as Administrator to fix Machine scope, or we will fix User scope now.');
+            envCheck.problems.forEach(p => logger.debug(`  - ${p}`));
             try {
                 await setupEnvironment(scope);
             } catch (e) {
diff --git a/src/system/service.ts b/src/system/service.ts
diff --git a/tests/system.test.ts b/tests/system.test.ts
