diff --git a/.husky/pre-push b/.husky/pre-push index 03f55f9..dc06c77 100755 --- a/.husky/pre-push +++ b/.husky/pre-push @@ -15,7 +15,7 @@ if [ -n "$HEAD_SHA" ] && [ "$HEAD_SHA" = "$CACHED_SHA" ]; then echo "Tests already passed for $HEAD_SHA — skipping." else echo "Running full test suite (unit + integration) before push..." - npm run test:all + npm run test:all || exit 1 fi echo "Checking for dependency vulnerabilities..." diff --git a/electron/ipc-setup.js b/electron/ipc-setup.js index a59d315..6410568 100644 --- a/electron/ipc-setup.js +++ b/electron/ipc-setup.js @@ -99,21 +99,28 @@ function registerSetupHandlers(ipcMain, getMainWindow) { }); ipcMain.handle('sidecar:get-api-keys', () => { - const { readApiKeys, readApiKeyHints, saveApiKey } = require('../src/utils/api-key-store'); - const { importFromAuthJson } = require('../src/utils/auth-json'); - const status = readApiKeys(); - const hints = readApiKeyHints(); + try { + const { readApiKeys, readApiKeyHints, saveApiKey } = require('../src/utils/api-key-store'); + const { importFromAuthJson } = require('../src/utils/auth-json'); + const status = readApiKeys(); + const hints = readApiKeyHints(); - // Auto-import keys from auth.json that sidecar doesn't have yet - const { imported } = importFromAuthJson(status); - for (const entry of imported) { - saveApiKey(entry.provider, entry.key); - status[entry.provider] = true; - const visible = entry.key.slice(0, 8); - hints[entry.provider] = visible + '\u2022'.repeat(Math.max(0, Math.min(entry.key.length - 8, 12))); - } + // Auto-import keys from auth.json that sidecar doesn't have yet + const { imported } = importFromAuthJson(status); + for (const entry of imported) { + const result = saveApiKey(entry.provider, entry.key); + if (result && result.success !== false) { + status[entry.provider] = true; + const visible = entry.key.slice(0, 8); + hints[entry.provider] = visible + '\u2022'.repeat(Math.max(0, Math.min(entry.key.length - 8, 12))); + } + } - return { status, hints, imported: imported.map(e => e.provider) }; + return { status, hints, imported: imported.map(e => e.provider) }; + } catch (err) { + logger.error('get-api-keys handler error', { error: err.message }); + return { status: {}, hints: {}, imported: [] }; + } }); ipcMain.handle('sidecar:fetch-models', async () => { diff --git a/src/cli.js b/src/cli.js index f928b68..4ab4061 100644 --- a/src/cli.js +++ b/src/cli.js @@ -47,15 +47,29 @@ function parseArgs(argv) { const arg = argv[i]; if (arg.startsWith('--')) { - const key = arg.slice(2); + let key = arg.slice(2); const next = argv[i + 1]; + // Handle --key=value syntax + let inlineValue; + const eqIdx = key.indexOf('='); + if (eqIdx !== -1) { + inlineValue = key.slice(eqIdx + 1); + key = key.slice(0, eqIdx); + } + // Boolean flags (no value expected) if (isBooleanFlag(key)) { result[key] = true; continue; } + // If --key=value was used, use the inline value directly + if (inlineValue !== undefined) { + result[key] = parseValue(key, inlineValue); + continue; + } + // Array accumulation flags if (key === 'exclude-mcp' && next && !next.startsWith('--')) { result['exclude-mcp'] = result['exclude-mcp'] || []; diff --git a/src/sidecar/setup-window.js b/src/sidecar/setup-window.js index 708a27c..1a4f984 100644 --- a/src/sidecar/setup-window.js +++ b/src/sidecar/setup-window.js @@ -29,13 +29,13 @@ function launchSetupWindow() { SIDECAR_MODE: 'setup' }; - const debugPort = process.env.SIDECAR_DEBUG_PORT || '9222'; - logger.info('Launching setup window', { debugPort }); + const debugPort = process.env.SIDECAR_DEBUG_PORT; + const args = debugPort + ? [`--remote-debugging-port=${debugPort}`, mainPath] + : [mainPath]; + logger.info('Launching setup window', { debugPort: debugPort || 'disabled' }); - const proc = spawn(electronPath, [ - `--remote-debugging-port=${debugPort}`, - mainPath - ], { + const proc = spawn(electronPath, args, { env, stdio: ['ignore', 'pipe', 'pipe'] }); diff --git a/src/utils/api-key-validation.js b/src/utils/api-key-validation.js index d640c16..8d3c6b8 100644 --- a/src/utils/api-key-validation.js +++ b/src/utils/api-key-validation.js @@ -60,6 +60,8 @@ function validateApiKey(provider, key) { if (provider === 'anthropic') { if (res.statusCode === 401) { resolve({ valid: false, error: 'Invalid API key (401)' }); + } else if (res.statusCode >= 500 || res.statusCode === 429) { + resolve({ valid: false, error: `Server error (${res.statusCode})` }); } else { resolve({ valid: true }); } @@ -75,6 +77,10 @@ function validateApiKey(provider, key) { } }); }); + req.setTimeout(10000, () => { + req.destroy(); + resolve({ valid: false, error: 'Request timed out' }); + }); req.on('error', (err) => { resolve({ valid: false, error: err.message }); }); diff --git a/tests/api-key-store-validation.test.js b/tests/api-key-store-validation.test.js index 8653313..bb6fb73 100644 --- a/tests/api-key-store-validation.test.js +++ b/tests/api-key-store-validation.test.js @@ -52,7 +52,7 @@ describe('api-key-store validation', () => { }; https.get.mockImplementation((_url, _opts, cb) => { cb(mockResponse); - return { on: jest.fn() }; + return { on: jest.fn(), setTimeout: jest.fn(), destroy: jest.fn() }; }); const result = await validateApiKey('openrouter', 'sk-or-valid'); @@ -70,7 +70,7 @@ describe('api-key-store validation', () => { }; https.get.mockImplementation((_url, _opts, cb) => { cb(mockResponse); - return { on: jest.fn() }; + return { on: jest.fn(), setTimeout: jest.fn(), destroy: jest.fn() }; }); const result = await validateApiKey('openrouter', 'sk-or-bad'); @@ -79,7 +79,7 @@ describe('api-key-store validation', () => { it('should resolve invalid for network error', async () => { https.get.mockImplementation((_url, _opts, _cb) => { - const req = { on: jest.fn() }; + const req = { on: jest.fn(), setTimeout: jest.fn(), destroy: jest.fn() }; setTimeout(() => { const errCall = req.on.mock.calls.find(c => c[0] === 'error'); if (errCall) { errCall[1](new Error('Network error')); } @@ -122,7 +122,7 @@ describe('api-key-store validation', () => { }; https.get.mockImplementation((_url, _opts, cb) => { cb(mockResponse); - return { on: jest.fn() }; + return { on: jest.fn(), setTimeout: jest.fn(), destroy: jest.fn() }; }); const result = await validateApiKey('openrouter', 'sk-or-forbidden'); @@ -140,7 +140,7 @@ describe('api-key-store validation', () => { }; https.get.mockImplementation((_url, _opts, cb) => { cb(mockResponse); - return { on: jest.fn() }; + return { on: jest.fn(), setTimeout: jest.fn(), destroy: jest.fn() }; }); const result = await validateApiKey('openrouter', 'sk-or-500'); @@ -159,7 +159,7 @@ describe('api-key-store validation', () => { }; https.get.mockImplementation((_url, _opts, cb) => { cb(mockResponse); - return { on: jest.fn() }; + return { on: jest.fn(), setTimeout: jest.fn(), destroy: jest.fn() }; }); const result = await validateApiKey('anthropic', 'sk-ant-valid'); @@ -177,13 +177,49 @@ describe('api-key-store validation', () => { }; https.get.mockImplementation((_url, _opts, cb) => { cb(mockResponse); - return { on: jest.fn() }; + return { on: jest.fn(), setTimeout: jest.fn(), destroy: jest.fn() }; }); const result = await validateApiKey('anthropic', 'sk-ant-bad'); expect(result).toEqual({ valid: false, error: 'Invalid API key (401)' }); }); + it('should treat anthropic 429 as invalid (rate limited)', async () => { + const mockResponse = { + statusCode: 429, + on: jest.fn((event, cb) => { + if (event === 'data') { cb('Rate limited'); } + if (event === 'end') { cb(); } + return mockResponse; + }) + }; + https.get.mockImplementation((_url, _opts, cb) => { + cb(mockResponse); + return { on: jest.fn(), setTimeout: jest.fn(), destroy: jest.fn() }; + }); + + const result = await validateApiKey('anthropic', 'sk-ant-ratelimited'); + expect(result).toEqual({ valid: false, error: 'Server error (429)' }); + }); + + it('should treat anthropic 500 as invalid (server error)', async () => { + const mockResponse = { + statusCode: 500, + on: jest.fn((event, cb) => { + if (event === 'data') { cb('Internal error'); } + if (event === 'end') { cb(); } + return mockResponse; + }) + }; + https.get.mockImplementation((_url, _opts, cb) => { + cb(mockResponse); + return { on: jest.fn(), setTimeout: jest.fn(), destroy: jest.fn() }; + }); + + const result = await validateApiKey('anthropic', 'sk-ant-servererror'); + expect(result).toEqual({ valid: false, error: 'Server error (500)' }); + }); + it('should validate deepseek key using correct endpoint', async () => { let capturedUrl; let capturedHeaders; @@ -199,7 +235,7 @@ describe('api-key-store validation', () => { capturedUrl = url; capturedHeaders = opts.headers; cb(mockResponse); - return { on: jest.fn() }; + return { on: jest.fn(), setTimeout: jest.fn(), destroy: jest.fn() }; }); const result = await validateApiKey('deepseek', 'sk-deepseek-valid'); @@ -221,7 +257,7 @@ describe('api-key-store validation', () => { https.get.mockImplementation((url, _opts, cb) => { capturedUrl = url; cb(mockResponse); - return { on: jest.fn() }; + return { on: jest.fn(), setTimeout: jest.fn(), destroy: jest.fn() }; }); await validateApiKey('google', 'AIza-test-key'); diff --git a/tests/cli.test.js b/tests/cli.test.js index 3f78200..f810788 100644 --- a/tests/cli.test.js +++ b/tests/cli.test.js @@ -348,6 +348,28 @@ describe('CLI Argument Parser', () => { expect(result['no-ui']).toBe(true); }); }); + + describe('--key=value syntax', () => { + it('should parse --model=gemini as model: "gemini"', () => { + const result = parseArgs(['start', '--model=gemini', '--prompt', 'test']); + expect(result.model).toBe('gemini'); + }); + + it('should parse --timeout=30 as numeric', () => { + const result = parseArgs(['start', '--timeout=30']); + expect(result.timeout).toBe(30); + }); + + it('should handle values containing equals signs', () => { + const result = parseArgs(['start', '--model=google/gemini-2.5-flash']); + expect(result.model).toBe('google/gemini-2.5-flash'); + }); + + it('should still support --model gemini (space-separated)', () => { + const result = parseArgs(['start', '--model', 'gemini']); + expect(result.model).toBe('gemini'); + }); + }); }); describe('validateStartArgs', () => { diff --git a/tests/sidecar/setup-window.test.js b/tests/sidecar/setup-window.test.js index e1b596e..aa38bc8 100644 --- a/tests/sidecar/setup-window.test.js +++ b/tests/sidecar/setup-window.test.js @@ -54,8 +54,8 @@ describe('setup-window', () => { expect(spawn).toHaveBeenCalled(); const spawnArgs = spawn.mock.calls[0]; - expect(spawnArgs[1][0]).toContain('--remote-debugging-port='); - expect(spawnArgs[1][1]).toContain('main.js'); + // Debug port is opt-in: without SIDECAR_DEBUG_PORT, only main.js is passed + expect(spawnArgs[1][0]).toContain('main.js'); const env = spawnArgs[2].env; expect(env.SIDECAR_MODE).toBe('setup'); @@ -63,6 +63,27 @@ describe('setup-window', () => { expect(result.success).toBe(true); }); + it('should pass --remote-debugging-port when SIDECAR_DEBUG_PORT is set', async () => { + process.env.SIDECAR_DEBUG_PORT = '9333'; + spawn.mockClear(); + spawn.mockReturnValue(mockProcess); + + const promise = launchSetupWindow(); + + const dataCallback = mockProcess.stdout.on.mock.calls.find(c => c[0] === 'data')[1]; + dataCallback('{"status":"complete"}\n'); + + const closeCallback = mockProcess.on.mock.calls.find(c => c[0] === 'close')[1]; + closeCallback(0); + + await promise; + + const spawnArgs = spawn.mock.calls[0]; + expect(spawnArgs[1][0]).toBe('--remote-debugging-port=9333'); + expect(spawnArgs[1][1]).toContain('main.js'); + delete process.env.SIDECAR_DEBUG_PORT; + }); + it('should parse enriched JSON with default model and keyCount', async () => { const promise = launchSetupWindow();