diff --git a/bin/happy-dev.mjs b/bin/happy-dev.mjs index 4a576e51..6fd3190d 100755 --- a/bin/happy-dev.mjs +++ b/bin/happy-dev.mjs @@ -9,15 +9,15 @@ import { homedir } from 'os'; const hasNoWarnings = process.execArgv.includes('--no-warnings'); const hasNoDeprecation = process.execArgv.includes('--no-deprecation'); +// Set development environment variables +process.env.HAPPY_HOME_DIR = join(homedir(), '.happy-dev'); +process.env.HAPPY_VARIANT = 'dev'; + if (!hasNoWarnings || !hasNoDeprecation) { // Re-execute with the flags const __filename = fileURLToPath(import.meta.url); const scriptPath = join(dirname(__filename), '../dist/index.mjs'); - // Set development environment variables - process.env.HAPPY_HOME_DIR = join(homedir(), '.happy-dev'); - process.env.HAPPY_VARIANT = 'dev'; - try { execFileSync( process.execPath, @@ -33,9 +33,5 @@ if (!hasNoWarnings || !hasNoDeprecation) { } } else { // Already have the flags, import normally - // Set development environment variables - process.env.HAPPY_HOME_DIR = join(homedir(), '.happy-dev'); - process.env.HAPPY_VARIANT = 'dev'; - await import('../dist/index.mjs'); } diff --git a/docs/bug-fix-plan-2025-01-15-athundt.md b/docs/bug-fix-plan-2025-01-15-athundt.md index 488f43cb..a3d7c61a 100644 --- a/docs/bug-fix-plan-2025-01-15-athundt.md +++ b/docs/bug-fix-plan-2025-01-15-athundt.md @@ -149,7 +149,6 @@ describe('claudeLocal --continue handling', () => { addListener: vi.fn(), removeListener: vi.fn(), kill: vi.fn(), - on: vi.fn(), stdout: { on: vi.fn() }, stderr: { on: vi.fn() }, stdin: { on: vi.fn(), end: vi.fn() } diff --git a/scripts/__tests__/ripgrep_launcher.test.ts b/scripts/__tests__/ripgrep_launcher.test.ts index 258dcb72..1ef2e254 100644 --- a/scripts/__tests__/ripgrep_launcher.test.ts +++ b/scripts/__tests__/ripgrep_launcher.test.ts @@ -89,6 +89,18 @@ describe('Ripgrep Launcher Runtime Compatibility', () => { expect(content).toContain('brew install ripgrep'); expect(content).toContain('winget install BurntSushi.ripgrep'); expect(content).toContain('Search functionality unavailable'); + expect(content).toContain('Missing arguments: expected JSON-encoded argv'); }).not.toThrow(); }); -}); \ No newline at end of file + + it('does not treat signal termination as success', () => { + expect(() => { + const fs = require('fs'); + const path = require('path'); + const content = fs.readFileSync(path.join(__dirname, '../ripgrep_launcher.cjs'), 'utf8'); + + expect(content).not.toContain('result.status || 0'); + expect(content).toContain('result.signal'); + }).not.toThrow(); + }); +}); diff --git a/scripts/claude_version_utils.cjs b/scripts/claude_version_utils.cjs index 2184917a..1daa0373 100644 --- a/scripts/claude_version_utils.cjs +++ b/scripts/claude_version_utils.cjs @@ -131,7 +131,9 @@ function detectSourceFromPath(resolvedPath) { // Windows-specific detection (detect by path patterns, not current platform) if (normalizedPath.includes('appdata') || normalizedPath.includes('program files') || normalizedPath.endsWith('.exe')) { // Windows npm - if (normalizedPath.includes('appdata') && normalizedPath.includes('npm') && normalizedPath.includes('node_modules')) { + if (normalizedPath.includes('appdata') && normalizedPath.includes('npm') && normalizedPath.includes('node_modules') && + normalizedPath.includes('@anthropic-ai') && normalizedPath.includes('claude-code') && + !normalizedPath.includes('.claude-code-')) { return 'npm'; } diff --git a/scripts/claude_version_utils.test.ts b/scripts/claude_version_utils.test.ts index d83e16ad..b5b2d561 100644 --- a/scripts/claude_version_utils.test.ts +++ b/scripts/claude_version_utils.test.ts @@ -32,9 +32,9 @@ describe('Claude Version Utils - Cross-Platform Detection', () => { expect(result).toBe('npm'); }); - it('should detect npm with different scoped packages', () => { + it('should not classify unrelated scoped npm packages as npm', () => { const result = detectSourceFromPath('C:/Users/test/AppData/Roaming/npm/node_modules/@babel/core/cli.js'); - expect(result).toBe('npm'); + expect(result).toBe('PATH'); }); it('should detect npm through Homebrew', () => { diff --git a/scripts/env-wrapper.cjs b/scripts/env-wrapper.cjs index 2ec562af..b485984f 100755 --- a/scripts/env-wrapper.cjs +++ b/scripts/env-wrapper.cjs @@ -51,6 +51,15 @@ if (!variant || !VARIANTS[variant]) { process.exit(1); } +if (!command) { + console.error('Usage: node scripts/env-wrapper.js [...args]'); + console.error(''); + console.error('Examples:'); + console.error(' node scripts/env-wrapper.js stable daemon start'); + console.error(' node scripts/env-wrapper.js dev auth login'); + process.exit(1); +} + const config = VARIANTS[variant]; // Create home directory if it doesn't exist diff --git a/scripts/ripgrep_launcher.cjs b/scripts/ripgrep_launcher.cjs index ab00d9a8..6ebd4940 100644 --- a/scripts/ripgrep_launcher.cjs +++ b/scripts/ripgrep_launcher.cjs @@ -40,7 +40,7 @@ function findSystemRipgrep() { try { const result = execFileSync(cmd, args, { encoding: 'utf8', - stdio: 'ignore' + stdio: ['ignore', 'pipe', 'ignore'] }); if (result) { @@ -93,7 +93,9 @@ function createRipgrepWrapper(binaryPath) { stdio: 'inherit', cwd: process.cwd() }); - return result.status || 0; + if (typeof result.status === 'number') return result.status; + if (result.signal) return 1; + return 0; } }; } @@ -170,6 +172,11 @@ const args = process.argv.slice(2); // Parse the JSON-encoded arguments let parsedArgs; try { + if (!args[0]) { + console.error('Missing arguments: expected JSON-encoded argv as the first parameter.'); + console.error('Example: node scripts/ripgrep_launcher.cjs \'["--version"]\''); + process.exit(1); + } parsedArgs = JSON.parse(args[0]); } catch (error) { console.error('Failed to parse arguments:', error.message); @@ -183,4 +190,4 @@ try { } catch (error) { console.error('Ripgrep error:', error.message); process.exit(1); -} \ No newline at end of file +} diff --git a/scripts/test-continue-fix.sh b/scripts/test-continue-fix.sh index f3735511..ea1ce1cf 100755 --- a/scripts/test-continue-fix.sh +++ b/scripts/test-continue-fix.sh @@ -14,7 +14,7 @@ echo echo "2. Testing session finder with current directory..." node -e " const { resolve, join } = require('path'); -const { readdirSync, statSync, readFileSync } = require('fs'); +const { readdirSync, statSync, readFileSync, existsSync } = require('fs'); const { homedir } = require('os'); const workingDirectory = process.cwd(); @@ -22,6 +22,11 @@ const projectId = resolve(workingDirectory).replace(/[\\\\\\/\.:]/g, '-'); const claudeConfigDir = process.env.CLAUDE_CONFIG_DIR || join(homedir(), '.claude'); const projectDir = join(claudeConfigDir, 'projects', projectId); +if (!existsSync(projectDir)) { + console.log('ERROR: Project directory does not exist:', projectDir); + process.exit(1); +} + const uuidPattern = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}\$/i; const files = readdirSync(projectDir) diff --git a/src/api/api.test.ts b/src/api/api.test.ts index ab734325..8a42c5eb 100644 --- a/src/api/api.test.ts +++ b/src/api/api.test.ts @@ -176,64 +176,74 @@ describe('Api server error handling', () => { connectionState.reset(); const consoleSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); - // Mock axios to return 500 error - mockPost.mockRejectedValue({ - response: { status: 500 }, - isAxiosError: true - }); - - const result = await api.getOrCreateSession({ - tag: 'test-tag', - metadata: testMetadata, - state: null - }); - - expect(result).toBeNull(); - expect(consoleSpy).toHaveBeenCalledWith( - expect.stringContaining('⚠️ Happy server unreachable') - ); - consoleSpy.mockRestore(); + try { + // Mock axios to return 500 error + mockPost.mockRejectedValue({ + response: { status: 500 }, + isAxiosError: true + }); + + const result = await api.getOrCreateSession({ + tag: 'test-tag', + metadata: testMetadata, + state: null + }); + + expect(result).toBeNull(); + expect(consoleSpy).toHaveBeenCalledWith( + expect.stringContaining('⚠️ Happy server unreachable') + ); + } finally { + consoleSpy.mockRestore(); + } }); it('should return null when server returns 503 Service Unavailable', async () => { connectionState.reset(); const consoleSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); - // Mock axios to return 503 error - mockPost.mockRejectedValue({ - response: { status: 503 }, - isAxiosError: true - }); - - const result = await api.getOrCreateSession({ - tag: 'test-tag', - metadata: testMetadata, - state: null - }); - - expect(result).toBeNull(); - expect(consoleSpy).toHaveBeenCalledWith( - expect.stringContaining('⚠️ Happy server unreachable') - ); - consoleSpy.mockRestore(); + try { + // Mock axios to return 503 error + mockPost.mockRejectedValue({ + response: { status: 503 }, + isAxiosError: true + }); + + const result = await api.getOrCreateSession({ + tag: 'test-tag', + metadata: testMetadata, + state: null + }); + + expect(result).toBeNull(); + expect(consoleSpy).toHaveBeenCalledWith( + expect.stringContaining('⚠️ Happy server unreachable') + ); + } finally { + consoleSpy.mockRestore(); + } }); it('should re-throw non-connection errors', async () => { - // Mock axios to throw a different type of error (e.g., authentication error) - const authError = new Error('Invalid API key'); - (authError as any).code = 'UNAUTHORIZED'; - mockPost.mockRejectedValue(authError); - - await expect( - api.getOrCreateSession({ tag: 'test-tag', metadata: testMetadata, state: null }) - ).rejects.toThrow('Failed to get or create session: Invalid API key'); - - // Should not show the offline mode message const consoleSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); - expect(consoleSpy).not.toHaveBeenCalledWith( - expect.stringContaining('⚠️ Happy server unreachable') - ); - consoleSpy.mockRestore(); + + try { + // Mock axios to throw a different type of error (e.g., authentication error) + const authError = new Error('Invalid API key'); + (authError as any).code = 'UNAUTHORIZED'; + mockPost.mockRejectedValue(authError); + + await expect( + api.getOrCreateSession({ tag: 'test-tag', metadata: testMetadata, state: null }) + ).rejects.toThrow('Failed to get or create session: Invalid API key'); + + // Should not show the offline mode message + expect(consoleSpy).not.toHaveBeenCalledWith( + expect.stringContaining('⚠️ Happy server unreachable') + ); + } finally { + consoleSpy.mockRestore(); + } }); }); @@ -310,4 +320,4 @@ describe('Api server error handling', () => { consoleSpy.mockRestore(); }); }); -}); \ No newline at end of file +}); diff --git a/src/api/apiMachine.ts b/src/api/apiMachine.ts index b8e2b570..d7f691d8 100644 --- a/src/api/apiMachine.ts +++ b/src/api/apiMachine.ts @@ -102,14 +102,30 @@ export class ApiMachineClient { }: MachineRpcHandlers) { // Register spawn session handler this.rpcHandlerManager.registerHandler('spawn-happy-session', async (params: any) => { - const { directory, sessionId, machineId, approvedNewDirectoryCreation, agent, token, environmentVariables } = params || {}; - logger.debug(`[API MACHINE] Spawning session with params: ${JSON.stringify(params)}`); + const { directory, sessionId, machineId, approvedNewDirectoryCreation, agent, token, environmentVariables, profileId } = params || {}; + const envKeys = environmentVariables && typeof environmentVariables === 'object' + ? Object.keys(environmentVariables as Record) + : []; + const maxEnvKeysToLog = 20; + const envKeySample = envKeys.slice(0, maxEnvKeysToLog); + logger.debug('[API MACHINE] Spawning session', { + directory, + sessionId, + machineId, + agent, + approvedNewDirectoryCreation, + profileId, + hasToken: !!token, + environmentVariableCount: envKeys.length, + environmentVariableKeySample: envKeySample, + environmentVariableKeysTruncated: envKeys.length > maxEnvKeysToLog, + }); if (!directory) { throw new Error('Directory is required'); } - const result = await spawnSession({ directory, sessionId, machineId, approvedNewDirectoryCreation, agent, token, environmentVariables }); + const result = await spawnSession({ directory, sessionId, machineId, approvedNewDirectoryCreation, agent, token, environmentVariables, profileId }); switch (result.type) { case 'success': @@ -327,4 +343,4 @@ export class ApiMachineClient { logger.debug('[API MACHINE] Socket closed'); } } -} \ No newline at end of file +} diff --git a/src/api/apiSession.test.ts b/src/api/apiSession.test.ts index 977e9ee8..20f47b3b 100644 --- a/src/api/apiSession.test.ts +++ b/src/api/apiSession.test.ts @@ -1,5 +1,6 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { ApiSessionClient } from './apiSession'; +import type { RawJSONLines } from '@/claude/types'; // Use vi.hoisted to ensure mock function is available when vi.mock factory runs const { mockIo } = vi.hoisted(() => ({ @@ -20,10 +21,12 @@ describe('ApiSessionClient connection handling', () => { // Mock socket.io client mockSocket = { + connected: false, connect: vi.fn(), on: vi.fn(), off: vi.fn(), - disconnect: vi.fn() + disconnect: vi.fn(), + emit: vi.fn(), }; mockIo.mockReturnValue(mockSocket); @@ -65,8 +68,32 @@ describe('ApiSessionClient connection handling', () => { expect(mockSocket.on).toHaveBeenCalledWith('error', expect.any(Function)); }); + it('emits messages even when disconnected (socket.io will buffer)', () => { + mockSocket.connected = false; + + const client = new ApiSessionClient('fake-token', mockSession); + + const payload: RawJSONLines = { + type: 'user', + uuid: 'test-uuid', + message: { + content: 'hello', + }, + } as const; + + client.sendClaudeSessionMessage(payload); + + expect(mockSocket.emit).toHaveBeenCalledWith( + 'message', + expect.objectContaining({ + sid: mockSession.id, + message: expect.any(String), + }) + ); + }); + afterEach(() => { consoleSpy.mockRestore(); vi.restoreAllMocks(); }); -}); \ No newline at end of file +}); diff --git a/src/api/apiSession.ts b/src/api/apiSession.ts index 187ce5b8..bbde3b98 100644 --- a/src/api/apiSession.ts +++ b/src/api/apiSession.ts @@ -53,6 +53,16 @@ export class ApiSessionClient extends EventEmitter { private metadataLock = new AsyncLock(); private encryptionKey: Uint8Array; private encryptionVariant: 'legacy' | 'dataKey'; + private disconnectedSendLogged = false; + + private logSendWhileDisconnected(context: string, details?: Record): void { + if (this.socket.connected || this.disconnectedSendLogged) return; + this.disconnectedSendLogged = true; + logger.debug( + `[API] Socket not connected; emitting ${context} anyway (socket.io should buffer until reconnection).`, + details + ); + } constructor(token: string, session: Session) { super() @@ -100,6 +110,7 @@ export class ApiSessionClient extends EventEmitter { this.socket.on('connect', () => { logger.debug('Socket connected successfully'); + this.disconnectedSendLogged = false; this.rpcHandlerManager.onSocketConnect(this.socket); }) @@ -221,11 +232,7 @@ export class ApiSessionClient extends EventEmitter { logger.debugLargeJson('[SOCKET] Sending message through socket:', content) - // Check if socket is connected before sending - if (!this.socket.connected) { - logger.debug('[API] Socket not connected, cannot send Claude session message. Message will be lost:', { type: body.type }); - return; - } + this.logSendWhileDisconnected('Claude session message', { type: body.type }); const encrypted = encodeBase64(encrypt(this.encryptionKey, this.encryptionVariant, content)); this.socket.emit('message', { @@ -265,13 +272,10 @@ export class ApiSessionClient extends EventEmitter { sentFrom: 'cli' } }; - const encrypted = encodeBase64(encrypt(this.encryptionKey, this.encryptionVariant, content)); - // Check if socket is connected before sending - if (!this.socket.connected) { - logger.debug('[API] Socket not connected, cannot send message. Message will be lost:', { type: body.type }); - // TODO: Consider implementing message queue or HTTP fallback for reliability - } + this.logSendWhileDisconnected('Codex message', { type: body?.type }); + + const encrypted = encodeBase64(encrypt(this.encryptionKey, this.encryptionVariant, content)); this.socket.emit('message', { sid: this.sessionId, @@ -300,8 +304,9 @@ export class ApiSessionClient extends EventEmitter { }; logger.debug(`[SOCKET] Sending ACP message from ${provider}:`, { type: body.type, hasMessage: 'message' in body }); - + this.logSendWhileDisconnected(`${provider} ACP message`, { type: body.type }); const encrypted = encodeBase64(encrypt(this.encryptionKey, this.encryptionVariant, content)); + this.socket.emit('message', { sid: this.sessionId, message: encrypted @@ -325,7 +330,11 @@ export class ApiSessionClient extends EventEmitter { data: event } }; + + this.logSendWhileDisconnected('session event', { eventType: event.type }); + const encrypted = encodeBase64(encrypt(this.encryptionKey, this.encryptionVariant, content)); + this.socket.emit('message', { sid: this.sessionId, message: encrypted @@ -373,8 +382,7 @@ export class ApiSessionClient extends EventEmitter { cache_read: usage.cache_read_input_tokens || 0 }, cost: { - // TODO: Calculate actual costs based on pricing - // For now, using placeholder values + // Costs are not currently calculated (placeholder values). total: 0, input: 0, output: 0 diff --git a/src/api/types.ts b/src/api/types.ts index e1b4878d..ea37c49a 100644 --- a/src/api/types.ts +++ b/src/api/types.ts @@ -218,7 +218,7 @@ export type Machine = { id: string, encryptionKey: Uint8Array; encryptionVariant: 'legacy' | 'dataKey'; - metadata: MachineMetadata, + metadata: MachineMetadata | null, metadataVersion: number, daemonState: DaemonState | null, daemonStateVersion: number, @@ -305,6 +305,12 @@ export type Metadata = { version?: string, name?: string, os?: string, + /** + * Session-scoped profile identity (non-secret). + * Used for display/debugging across devices; runtime behavior is still driven by env vars at spawn. + * Null indicates "no profile". + */ + profileId?: string | null, summary?: { text: string, updatedAt: number diff --git a/src/claude/claudeLocal.ts b/src/claude/claudeLocal.ts index d4f7ac0b..b009cc6c 100644 --- a/src/claude/claudeLocal.ts +++ b/src/claude/claudeLocal.ts @@ -34,7 +34,7 @@ export async function claudeLocal(opts: { // Check if claudeArgs contains --continue or --resume (user passed these flags) const hasContinueFlag = opts.claudeArgs?.includes('--continue'); - const hasResumeFlag = opts.claudeArgs?.includes('--resume'); + const hasResumeFlag = opts.claudeArgs?.includes('--resume') || opts.claudeArgs?.includes('-r'); const hasUserSessionControl = hasContinueFlag || hasResumeFlag; // Determine if we have an existing session to resume @@ -172,7 +172,6 @@ export async function claudeLocal(opts: { // Session/resume args depend on whether we're in offline mode or hook mode if (!opts.hookSettingsPath) { // Offline mode: We control session ID - const hasResumeFlag = opts.claudeArgs?.includes('--resume') || opts.claudeArgs?.includes('-r'); if (startFrom) { // Resume existing session (Claude preserves the session ID) args.push('--resume', startFrom) diff --git a/src/claude/runClaude.ts b/src/claude/runClaude.ts index bcdd74fd..dc07d2c2 100644 --- a/src/claude/runClaude.ts +++ b/src/claude/runClaude.ts @@ -83,11 +83,15 @@ export async function runClaude(credentials: Credentials, options: StartOptions metadata: initialMachineMetadata }); + const profileIdEnv = process.env.HAPPY_SESSION_PROFILE_ID; + const profileId = profileIdEnv === undefined ? undefined : (profileIdEnv.trim() || null); + let metadata: Metadata = { path: workingDirectory, host: os.hostname(), version: packageJson.version, os: os.platform(), + ...(profileIdEnv !== undefined ? { profileId } : {}), machineId: machineId, homeDir: os.homedir(), happyHomeDir: configuration.happyHomeDir, @@ -128,19 +132,26 @@ export async function runClaude(credentials: Credentials, options: StartOptions } }); + const abortController = new AbortController(); + const abortOnSignal = () => abortController.abort(); + process.once('SIGINT', abortOnSignal); + process.once('SIGTERM', abortOnSignal); + try { await claudeLocal({ path: workingDirectory, sessionId: null, onSessionFound: (id) => { offlineSessionId = id; }, onThinkingChange: () => {}, - abort: new AbortController().signal, + abort: abortController.signal, claudeEnvVars: options.claudeEnvVars, claudeArgs: options.claudeArgs, mcpServers: {}, allowedTools: [] }); } finally { + process.removeListener('SIGINT', abortOnSignal); + process.removeListener('SIGTERM', abortOnSignal); reconnection.cancel(); stopCaffeinate(); } @@ -490,4 +501,4 @@ export async function runClaude(credentials: Credentials, options: StartOptions // Exit process.exit(0); -} \ No newline at end of file +} diff --git a/src/claude/utils/sessionScanner.test.ts b/src/claude/utils/sessionScanner.test.ts index 91920523..1535b410 100644 --- a/src/claude/utils/sessionScanner.test.ts +++ b/src/claude/utils/sessionScanner.test.ts @@ -3,21 +3,37 @@ import { createSessionScanner } from './sessionScanner' import { RawJSONLines } from '../types' import { mkdir, writeFile, appendFile, rm, readFile } from 'node:fs/promises' import { join } from 'node:path' -import { tmpdir, homedir } from 'node:os' +import { tmpdir } from 'node:os' import { existsSync } from 'node:fs' +import { getProjectPath } from './path' + +async function waitFor(predicate: () => boolean, timeoutMs: number = 2000, intervalMs: number = 25): Promise { + const start = Date.now() + while (Date.now() - start < timeoutMs) { + if (predicate()) return + await new Promise(resolve => setTimeout(resolve, intervalMs)) + } + throw new Error('Timed out waiting for condition') +} describe('sessionScanner', () => { let testDir: string let projectDir: string let collectedMessages: RawJSONLines[] let scanner: Awaited> | null = null + let originalClaudeConfigDir: string | undefined + let claudeConfigDir: string beforeEach(async () => { testDir = join(tmpdir(), `scanner-test-${Date.now()}`) await mkdir(testDir, { recursive: true }) - const projectName = testDir.replace(/\//g, '-') - projectDir = join(homedir(), '.claude', 'projects', projectName) + // Ensure the scanner and this test agree on where session files live. + // (getProjectPath uses CLAUDE_CONFIG_DIR + a sanitized project id derived from workingDirectory.) + originalClaudeConfigDir = process.env.CLAUDE_CONFIG_DIR; + claudeConfigDir = join(testDir, 'claude-config'); + process.env.CLAUDE_CONFIG_DIR = claudeConfigDir; + projectDir = getProjectPath(testDir); await mkdir(projectDir, { recursive: true }) collectedMessages = [] @@ -30,12 +46,15 @@ describe('sessionScanner', () => { scanner = null } + if (originalClaudeConfigDir === undefined) { + delete process.env.CLAUDE_CONFIG_DIR; + } else { + process.env.CLAUDE_CONFIG_DIR = originalClaudeConfigDir; + } + if (existsSync(testDir)) { await rm(testDir, { recursive: true, force: true }) } - if (existsSync(projectDir)) { - await rm(projectDir, { recursive: true, force: true }) - } }) it('should process initial session and resumed session correctly', async () => { @@ -63,7 +82,7 @@ describe('sessionScanner', () => { // Write first line await writeFile(sessionFile1, lines1[0] + '\n') scanner.onNewSession(sessionId1) - await new Promise(resolve => setTimeout(resolve, 100)) + await waitFor(() => collectedMessages.length >= 1) expect(collectedMessages).toHaveLength(1) expect(collectedMessages[0].type).toBe('user') @@ -76,7 +95,7 @@ describe('sessionScanner', () => { // Write second line with delay await new Promise(resolve => setTimeout(resolve, 50)) await appendFile(sessionFile1, lines1[1] + '\n') - await new Promise(resolve => setTimeout(resolve, 200)) + await waitFor(() => collectedMessages.length >= 2) expect(collectedMessages).toHaveLength(2) expect(collectedMessages[1].type).toBe('assistant') @@ -102,7 +121,7 @@ describe('sessionScanner', () => { await writeFile(sessionFile2, initialContent) scanner.onNewSession(sessionId2) - await new Promise(resolve => setTimeout(resolve, 100)) + await waitFor(() => collectedMessages.length >= phase1Count + 1) // Should have added only 1 new message (summary) // The historical user + assistant messages (lines 1-2) are deduplicated because they have same UUIDs @@ -112,7 +131,7 @@ describe('sessionScanner', () => { // Write new messages (user asks for ls tool) - this is line 3 await new Promise(resolve => setTimeout(resolve, 50)) await appendFile(sessionFile2, lines2[3] + '\n') - await new Promise(resolve => setTimeout(resolve, 200)) + await waitFor(() => collectedMessages.some(m => m.type === 'user' && m.message.content === 'run ls tool ')) // Find the user message we just added const userMessages = collectedMessages.filter(m => m.type === 'user') @@ -127,7 +146,12 @@ describe('sessionScanner', () => { await new Promise(resolve => setTimeout(resolve, 50)) await appendFile(sessionFile2, lines2[i] + '\n') } - await new Promise(resolve => setTimeout(resolve, 300)) + await waitFor(() => collectedMessages.some(m => + m.type === 'assistant' && + Array.isArray((m.message?.content as any)) && + typeof (m.message?.content as any)[0]?.text === 'string' && + (m.message?.content as any)[0].text.includes('0-say-lol-session.jsonl') + ), 5000) // Final count check const finalMessages = collectedMessages.slice(phase1Count) @@ -207,4 +231,4 @@ describe('sessionScanner', () => { // expect(lastAssistantMsg.message.id).toBe('msg_01KWeuP88pkzRtXmggJRnQmV') // } }) -}) \ No newline at end of file +}) diff --git a/src/codex/codexMcpClient.test.ts b/src/codex/codexMcpClient.test.ts new file mode 100644 index 00000000..1e25c47f --- /dev/null +++ b/src/codex/codexMcpClient.test.ts @@ -0,0 +1,93 @@ +import { describe, it, expect, vi } from 'vitest'; +import type { CodexPermissionHandler } from './utils/permissionHandler'; +import { createCodexElicitationRequestHandler } from './codexMcpClient'; + +// NOTE: This test suite uses mocks because the real Codex CLI / MCP transport +// is not guaranteed to be available in CI or local test environments. +vi.mock('child_process', () => ({ + execSync: vi.fn(), +})); + +vi.mock('@modelcontextprotocol/sdk/types.js', () => ({ + ElicitRequestSchema: {}, +})); + +vi.mock('@modelcontextprotocol/sdk/client/stdio.js', () => { + const instances: any[] = []; + + class StdioClientTransport { + public command: string; + public args: string[]; + public env: Record; + + constructor(opts: { command: string; args: string[]; env: Record }) { + this.command = opts.command; + this.args = opts.args; + this.env = opts.env; + instances.push(this); + } + } + + return { StdioClientTransport, __transportInstances: instances }; +}); + +vi.mock('@modelcontextprotocol/sdk/client/index.js', () => { + class Client { + setNotificationHandler() { } + setRequestHandler() { } + async connect() { } + async close() { } + } + + return { Client }; +}); + +describe('CodexMcpClient elicitation handling', () => { + it('does not print elicitation payloads to stdout', async () => { + const consoleSpy = vi.spyOn(console, 'log').mockImplementation(() => {}); + + try { + consoleSpy.mockClear(); + + const permissionHandler = { + handleToolCall: vi.fn().mockResolvedValue({ decision: 'approved' }), + } as unknown as CodexPermissionHandler; + + const handler = createCodexElicitationRequestHandler(permissionHandler); + await handler({ + params: { + codex_call_id: 'call-1', + codex_command: 'echo hi', + codex_cwd: '/tmp', + }, + }); + + expect(consoleSpy).not.toHaveBeenCalled(); + expect(permissionHandler.handleToolCall).toHaveBeenCalled(); + } finally { + consoleSpy.mockRestore(); + } + }); +}); + +describe('CodexMcpClient command detection', () => { + it('does not treat "codex " output as "not installed"', async () => { + vi.resetModules(); + + const { execSync } = await import('child_process'); + (execSync as any).mockReturnValue('codex 0.43.0-alpha.5\n'); + + const stdioModule = (await import('@modelcontextprotocol/sdk/client/stdio.js')) as any; + const __transportInstances = stdioModule.__transportInstances as any[]; + __transportInstances.splice(0); + + const mod = await import('./codexMcpClient'); + + const client = new (mod as any).CodexMcpClient(); + await expect(client.connect()).resolves.toBeUndefined(); + + expect(__transportInstances.length).toBe(1); + expect(__transportInstances[0].command).toBe('codex'); + expect(__transportInstances[0].args).toEqual(['mcp-server']); + }); +}); diff --git a/src/codex/codexMcpClient.ts b/src/codex/codexMcpClient.ts index ed101394..2b0cbb99 100644 --- a/src/codex/codexMcpClient.ts +++ b/src/codex/codexMcpClient.ts @@ -21,10 +21,12 @@ const DEFAULT_TIMEOUT = 14 * 24 * 60 * 60 * 1000; // 14 days, which is the half function getCodexMcpCommand(): string | null { try { const version = execSync('codex --version', { encoding: 'utf8' }).trim(); - const match = version.match(/codex-cli\s+(\d+\.\d+\.\d+(?:-alpha\.\d+)?)/); + const match = version.match(/\b(?:codex-cli|codex)\s+v?(\d+\.\d+\.\d+(?:-alpha\.\d+)?)\b/i); if (!match) { logger.debug('[CodexMCP] Could not parse codex version:', version); - return null; + // If codex is installed but version format is unexpected, prefer the modern subcommand. + // Older versions may require 'mcp', but treating codex as "not installed" is misleading. + return 'mcp-server'; } const versionStr = match[1]; @@ -47,6 +49,76 @@ function getCodexMcpCommand(): string | null { } } +type CodexPermissionHandlerProvider = + | CodexPermissionHandler + | null + | (() => CodexPermissionHandler | null); + +const CodexBashElicitationParamsSchema = z + .object({ + codex_call_id: z.string(), + codex_command: z.string(), + codex_cwd: z.string().optional(), + }) + .passthrough(); + +type CodexBashElicitationParams = z.infer; + +export function createCodexElicitationRequestHandler( + permissionHandlerProvider: CodexPermissionHandlerProvider, +): (request: { params: unknown }) => Promise<{ decision: 'denied' | 'approved' | 'approved_for_session' | 'abort'; reason?: string }> { + const getPermissionHandler = + typeof permissionHandlerProvider === 'function' + ? permissionHandlerProvider + : () => permissionHandlerProvider; + + return async (request: { params: unknown }) => { + const permissionHandler = getPermissionHandler(); + const parsedParams = CodexBashElicitationParamsSchema.safeParse(request.params); + + const toolName = 'CodexBash'; + + if (!permissionHandler) { + logger.debug('[CodexMCP] No permission handler set, denying by default'); + return { + decision: 'denied' as const, + }; + } + + if (!parsedParams.success) { + logger.debug('[CodexMCP] Invalid elicitation params, denying by default', parsedParams.error.issues); + return { + decision: 'denied' as const, + reason: 'Invalid elicitation params', + }; + } + + const params: CodexBashElicitationParams = parsedParams.data; + + try { + const result = await permissionHandler.handleToolCall( + params.codex_call_id, + toolName, + { + command: params.codex_command, + cwd: params.codex_cwd, + }, + ); + + logger.debug('[CodexMCP] Permission result:', result); + return { + decision: result.decision, + }; + } catch (error) { + logger.debug('[CodexMCP] Error handling permission request:', error); + return { + decision: 'denied' as const, + reason: error instanceof Error ? error.message : 'Permission request failed', + }; + } + }; +} + export class CodexMcpClient { private client: Client; private transport: StdioClientTransport | null = null; @@ -125,55 +197,7 @@ export class CodexMcpClient { private registerPermissionHandlers(): void { // Register handler for exec command approval requests - this.client.setRequestHandler( - ElicitRequestSchema, - async (request) => { - console.log('[CodexMCP] Received elicitation request:', request.params); - - // Load params - const params = request.params as unknown as { - message: string, - codex_elicitation: string, - codex_mcp_tool_call_id: string, - codex_event_id: string, - codex_call_id: string, - codex_command: string[], - codex_cwd: string - } - const toolName = 'CodexBash'; - - // If no permission handler set, deny by default - if (!this.permissionHandler) { - logger.debug('[CodexMCP] No permission handler set, denying by default'); - return { - decision: 'denied' as const, - }; - } - - try { - // Request permission through the handler - const result = await this.permissionHandler.handleToolCall( - params.codex_call_id, - toolName, - { - command: params.codex_command, - cwd: params.codex_cwd - } - ); - - logger.debug('[CodexMCP] Permission result:', result); - return { - decision: result.decision - } - } catch (error) { - logger.debug('[CodexMCP] Error handling permission request:', error); - return { - decision: 'denied' as const, - reason: error instanceof Error ? error.message : 'Permission request failed' - }; - } - } - ); + this.client.setRequestHandler(ElicitRequestSchema, createCodexElicitationRequestHandler(() => this.permissionHandler)); logger.debug('[CodexMCP] Permission handlers registered'); } diff --git a/src/daemon/run.tmuxEnv.test.ts b/src/daemon/run.tmuxEnv.test.ts new file mode 100644 index 00000000..7f13a0c3 --- /dev/null +++ b/src/daemon/run.tmuxEnv.test.ts @@ -0,0 +1,16 @@ +import { describe, it, expect } from 'vitest'; + +describe('daemon tmux env building', () => { + it('merges daemon process env and profile env for tmux windows', async () => { + const runModule = (await import('@/daemon/run')) as typeof import('@/daemon/run'); + const merged = runModule.buildTmuxWindowEnv( + { PATH: '/bin', HOME: '/home/user', UNDEFINED: undefined }, + { HOME: '/override', CUSTOM: 'x' } + ); + + expect(merged.PATH).toBe('/bin'); + expect(merged.HOME).toBe('/override'); + expect(merged.CUSTOM).toBe('x'); + expect('UNDEFINED' in merged).toBe(false); + }, 15000); +}); diff --git a/src/daemon/run.tmuxSpawn.test.ts b/src/daemon/run.tmuxSpawn.test.ts new file mode 100644 index 00000000..c9b6bd4d --- /dev/null +++ b/src/daemon/run.tmuxSpawn.test.ts @@ -0,0 +1,39 @@ +import { describe, it, expect } from 'vitest'; + +describe('daemon tmux spawn config', () => { + const originalRuntimeOverride = process.env.HAPPY_CLI_SUBPROCESS_RUNTIME; + const originalPath = process.env.PATH; + + it('uses merged env and bun runtime when configured', async () => { + process.env.HAPPY_CLI_SUBPROCESS_RUNTIME = 'bun'; + process.env.PATH = '/bin'; + + try { + const runModule = (await import('@/daemon/run')) as typeof import('@/daemon/run'); + const cfg = runModule.buildTmuxSpawnConfig({ + agent: 'claude', + directory: '/tmp', + extraEnv: { + FOO: 'bar', + TMUX_TMPDIR: '/custom/tmux', + }, + }); + + expect(cfg.commandTokens[0]).toBe('bun'); + expect(cfg.tmuxEnv.PATH).toBe('/bin'); + expect(cfg.tmuxEnv.FOO).toBe('bar'); + expect(cfg.tmuxCommandEnv.TMUX_TMPDIR).toBe('/custom/tmux'); + } finally { + if (originalRuntimeOverride === undefined) { + delete process.env.HAPPY_CLI_SUBPROCESS_RUNTIME; + } else { + process.env.HAPPY_CLI_SUBPROCESS_RUNTIME = originalRuntimeOverride; + } + if (originalPath === undefined) { + delete process.env.PATH; + } else { + process.env.PATH = originalPath; + } + } + }, 15000); +}); diff --git a/src/daemon/run.ts b/src/daemon/run.ts index 75889d14..499bf5d0 100644 --- a/src/daemon/run.ts +++ b/src/daemon/run.ts @@ -12,15 +12,15 @@ import { configuration } from '@/configuration'; import { startCaffeinate, stopCaffeinate } from '@/utils/caffeinate'; import packageJson from '../../package.json'; import { getEnvironmentInfo } from '@/ui/doctor'; -import { spawnHappyCLI } from '@/utils/spawnHappyCLI'; -import { writeDaemonState, DaemonLocallyPersistedState, readDaemonState, acquireDaemonLock, releaseDaemonLock, readSettings, getActiveProfile, getEnvironmentVariables, validateProfileForAgent, getProfileEnvironmentVariables } from '@/persistence'; +import { buildHappyCliSubprocessInvocation, spawnHappyCLI } from '@/utils/spawnHappyCLI'; +import { writeDaemonState, DaemonLocallyPersistedState, readDaemonState, acquireDaemonLock, releaseDaemonLock, readSettings } from '@/persistence'; import { cleanupDaemonState, isDaemonRunningCurrentlyInstalledHappyVersion, stopDaemon } from './controlClient'; import { startDaemonControlServer } from './controlServer'; import { readFileSync } from 'fs'; import { join } from 'path'; import { projectPath } from '@/projectPath'; -import { getTmuxUtilities, isTmuxAvailable, parseTmuxSessionIdentifier, formatTmuxSessionIdentifier } from '@/utils/tmux'; +import { TmuxUtilities, isTmuxAvailable } from '@/utils/tmux'; import { expandEnvironmentVariables } from '@/utils/expandEnvVars'; // Prepare initial metadata @@ -33,35 +33,52 @@ export const initialMachineMetadata: MachineMetadata = { happyLibDir: projectPath() }; -// Get environment variables for a profile, filtered for agent compatibility -async function getProfileEnvironmentVariablesForAgent( - profileId: string, - agentType: 'claude' | 'codex' | 'gemini' -): Promise> { - try { - const settings = await readSettings(); - const profile = settings.profiles.find(p => p.id === profileId); - - if (!profile) { - logger.debug(`[DAEMON RUN] Profile ${profileId} not found`); - return {}; - } - - // Check if profile is compatible with the agent - if (!validateProfileForAgent(profile, agentType)) { - logger.debug(`[DAEMON RUN] Profile ${profileId} not compatible with agent ${agentType}`); - return {}; - } +export function buildTmuxWindowEnv( + daemonEnv: NodeJS.ProcessEnv, + extraEnv: Record, +): Record { + const filteredDaemonEnv = Object.fromEntries( + Object.entries(daemonEnv).filter(([, value]) => typeof value === 'string'), + ) as Record; - // Get environment variables from profile (new schema) - const envVars = getProfileEnvironmentVariables(profile); + return { ...filteredDaemonEnv, ...extraEnv }; +} - logger.debug(`[DAEMON RUN] Loaded ${Object.keys(envVars).length} environment variables from profile ${profileId} for agent ${agentType}`); - return envVars; - } catch (error) { - logger.debug('[DAEMON RUN] Failed to get profile environment variables:', error); - return {}; +export function buildTmuxSpawnConfig(params: { + agent: 'claude' | 'codex' | 'gemini'; + directory: string; + extraEnv: Record; +}): { + commandTokens: string[]; + tmuxEnv: Record; + tmuxCommandEnv: Record; + directory: string; +} { + const args = [ + params.agent, + '--happy-starting-mode', + 'remote', + '--started-by', + 'daemon', + ]; + + const { runtime, argv } = buildHappyCliSubprocessInvocation(args); + const commandTokens = [runtime, ...argv]; + + const tmuxEnv = buildTmuxWindowEnv(process.env, params.extraEnv); + + const tmuxCommandEnv: Record = {}; + const tmuxTmpDir = params.extraEnv.TMUX_TMPDIR; + if (typeof tmuxTmpDir === 'string' && tmuxTmpDir.length > 0) { + tmuxCommandEnv.TMUX_TMPDIR = tmuxTmpDir; } + + return { + commandTokens, + tmuxEnv, + tmuxCommandEnv, + directory: params.directory, + }; } export async function startDaemon(): Promise { @@ -166,6 +183,7 @@ export async function startDaemon(): Promise { // Setup state - key by PID const pidToTrackedSession = new Map(); + const codexHomeDirCleanupByPid = new Map void>(); // Session spawning awaiter system const pidToAwaiter = new Map void>(); @@ -217,11 +235,28 @@ export async function startDaemon(): Promise { // Spawn a new session (sessionId reserved for future --resume functionality) const spawnSession = async (options: SpawnSessionOptions): Promise => { - logger.debugLargeJson('[DAEMON RUN] Spawning session', options); + // Do NOT log raw options: it may include secrets (token / env vars). + const envKeys = options.environmentVariables && typeof options.environmentVariables === 'object' + ? Object.keys(options.environmentVariables as Record) + : []; + logger.debugLargeJson('[DAEMON RUN] Spawning session', { + directory: options.directory, + sessionId: options.sessionId, + machineId: options.machineId, + approvedNewDirectoryCreation: options.approvedNewDirectoryCreation, + agent: options.agent, + profileId: options.profileId, + hasToken: !!options.token, + environmentVariableCount: envKeys.length, + environmentVariableKeys: envKeys, + }); const { directory, sessionId, machineId, approvedNewDirectoryCreation = true } = options; let directoryCreated = false; + let codexHomeDirCleanup: (() => void) | null = null; + let codexHomeDirCleanupArmed = false; + try { await fs.access(directory); logger.debug(`[DAEMON RUN] Directory exists: ${directory}`); @@ -279,9 +314,10 @@ export async function startDaemon(): Promise { // Create a temporary directory for Codex const codexHomeDir = tmp.dirSync(); + codexHomeDirCleanup = codexHomeDir.removeCallback; // Write the token to the temporary directory - fs.writeFile(join(codexHomeDir.name, 'auth.json'), options.token); + await fs.writeFile(join(codexHomeDir.name, 'auth.json'), options.token); // Set the environment variable for Codex authEnv.CODEX_HOME = codexHomeDir.name; @@ -291,7 +327,9 @@ export async function startDaemon(): Promise { } // Layer 2: Profile environment variables - // Priority: GUI-provided profile > CLI local active profile > none + // IMPORTANT: only apply profile env when explicitly provided by the caller. + // We do NOT fall back to CLI-local active profile here, because sessions spawned via + // the daemon are typically requested by the GUI and must respect GUI opt-in gating. let profileEnv: Record = {}; if (options.environmentVariables && Object.keys(options.environmentVariables).length > 0) { @@ -300,31 +338,18 @@ export async function startDaemon(): Promise { logger.info(`[DAEMON RUN] Using GUI-provided profile environment variables (${Object.keys(profileEnv).length} vars)`); logger.debug(`[DAEMON RUN] GUI profile env var keys: ${Object.keys(profileEnv).join(', ')}`); } else { - // Fallback to CLI local active profile - try { - const settings = await readSettings(); - if (settings.activeProfileId) { - logger.debug(`[DAEMON RUN] No GUI profile provided, loading CLI local active profile: ${settings.activeProfileId}`); - - // Get profile environment variables filtered for agent compatibility - profileEnv = await getProfileEnvironmentVariablesForAgent( - settings.activeProfileId, - options.agent || 'claude' - ); - - logger.debug(`[DAEMON RUN] Loaded ${Object.keys(profileEnv).length} environment variables from CLI local profile for agent ${options.agent || 'claude'}`); - logger.debug(`[DAEMON RUN] CLI profile env var keys: ${Object.keys(profileEnv).join(', ')}`); - } else { - logger.debug('[DAEMON RUN] No CLI local active profile set'); - } - } catch (error) { - logger.debug('[DAEMON RUN] Failed to load CLI local profile environment variables:', error); - // Continue without profile env vars - this is not a fatal error - } + logger.debug('[DAEMON RUN] No profile environment variables provided by caller; skipping profile env injection'); } - // Final merge: Profile vars first, then auth (auth takes precedence to protect authentication) - let extraEnv = { ...profileEnv, ...authEnv }; + // Session identity (non-secret) for cross-device display/debugging + // Empty string means "no profile" and should still be preserved. + const sessionProfileEnv: Record = {}; + if (options.profileId !== undefined) { + sessionProfileEnv.HAPPY_SESSION_PROFILE_ID = options.profileId; + } + + // Final merge: profile vars + session identity, then auth (auth takes precedence to protect authentication) + let extraEnv = { ...profileEnv, ...sessionProfileEnv, ...authEnv }; logger.debug(`[DAEMON RUN] Final environment variable keys (before expansion) (${Object.keys(extraEnv).length}): ${Object.keys(extraEnv).join(', ')}`); // Expand ${VAR} references from daemon's process.env @@ -354,6 +379,10 @@ export async function startDaemon(): Promise { const errorMessage = `Authentication will fail - environment variables not found in daemon: ${missingVarDetails.join('; ')}. ` + `Ensure these variables are set in the daemon's environment (not just your shell) before starting sessions.`; logger.warn(`[DAEMON RUN] ${errorMessage}`); + if (codexHomeDirCleanup && !codexHomeDirCleanupArmed) { + codexHomeDirCleanup(); + codexHomeDirCleanup = null; + } return { type: 'error', errorMessage @@ -382,33 +411,19 @@ export async function startDaemon(): Promise { const sessionDesc = tmuxSessionName || 'current/most recent session'; logger.debug(`[DAEMON RUN] Attempting to spawn session in tmux: ${sessionDesc}`); - const tmux = getTmuxUtilities(tmuxSessionName); - - // Construct command for the CLI - const cliPath = join(projectPath(), 'dist', 'index.mjs'); // Determine agent command - support claude, codex, and gemini const agent = options.agent === 'gemini' ? 'gemini' : (options.agent === 'codex' ? 'codex' : 'claude'); - const fullCommand = `node --no-warnings --no-deprecation ${cliPath} ${agent} --happy-starting-mode remote --started-by daemon`; + const { commandTokens, tmuxEnv, tmuxCommandEnv } = buildTmuxSpawnConfig({ agent, directory, extraEnv }); + const tmux = new TmuxUtilities(tmuxSessionName, tmuxCommandEnv); // Spawn in tmux with environment variables - // IMPORTANT: Pass complete environment (process.env + extraEnv) because: - // 1. tmux sessions need daemon's expanded auth variables (e.g., ANTHROPIC_AUTH_TOKEN) - // 2. Regular spawn uses env: { ...process.env, ...extraEnv } - // 3. tmux needs explicit environment via -e flags to ensure all variables are available + // IMPORTANT: `spawnInTmux` uses `-e KEY=VALUE` flags for the window. + // Use merged env so tmux mode matches regular process spawn behavior. + // Note: this may add many `-e` flags; if it becomes a problem we can optimize + // by diffing against `tmux show-environment` in a follow-up. const windowName = `happy-${Date.now()}-${agent}`; - const tmuxEnv: Record = {}; - - // Add all daemon environment variables (filtering out undefined) - for (const [key, value] of Object.entries(process.env)) { - if (value !== undefined) { - tmuxEnv[key] = value; - } - } - - // Add extra environment variables (these should already be filtered) - Object.assign(tmuxEnv, extraEnv); - const tmuxResult = await tmux.spawnInTmux([fullCommand], { + const tmuxResult = await tmux.spawnInTmux(commandTokens, { sessionName: tmuxSessionName, windowName: windowName, cwd: directory @@ -422,6 +437,9 @@ export async function startDaemon(): Promise { throw new Error('Tmux window created but no PID returned'); } + // Resolve the actual tmux session name used (important when sessionName was empty/undefined) + const tmuxSession = tmuxResult.sessionName ?? (tmuxSessionName || 'happy'); + // Create a tracked session for tmux windows - now we have the real PID! const trackedSession: TrackedSession = { startedBy: 'daemon', @@ -429,12 +447,16 @@ export async function startDaemon(): Promise { tmuxSessionId: tmuxResult.sessionId, directoryCreated, message: directoryCreated - ? `The path '${directory}' did not exist. We created a new folder and spawned a new session in tmux session '${tmuxSessionName}'. Use 'tmux attach -t ${tmuxSessionName}' to view the session.` - : `Spawned new session in tmux session '${tmuxSessionName}'. Use 'tmux attach -t ${tmuxSessionName}' to view the session.` + ? `The path '${directory}' did not exist. We created a new folder and spawned a new session in tmux session '${tmuxSession}'. Use 'tmux attach -t ${tmuxSession}' to view the session.` + : `Spawned new session in tmux session '${tmuxSession}'. Use 'tmux attach -t ${tmuxSession}' to view the session.` }; // Add to tracking map so webhook can find it later pidToTrackedSession.set(tmuxResult.pid, trackedSession); + if (codexHomeDirCleanup) { + codexHomeDirCleanupByPid.set(tmuxResult.pid, codexHomeDirCleanup); + codexHomeDirCleanupArmed = true; + } // Wait for webhook to populate session with happySessionId (exact same as regular flow) logger.debug(`[DAEMON RUN] Waiting for session webhook for PID ${tmuxResult.pid} (tmux)`); @@ -495,8 +517,7 @@ export async function startDaemon(): Promise { '--started-by', 'daemon' ]; - // TODO: In future, sessionId could be used with --resume to continue existing sessions - // For now, we ignore it - each spawn creates a new session + // Note: sessionId is not currently used to resume sessions; each spawn creates a new session. const happyProcess = spawnHappyCLI(args, { cwd: directory, detached: true, // Sessions stay alive when daemon stops @@ -519,6 +540,10 @@ export async function startDaemon(): Promise { if (!happyProcess.pid) { logger.debug('[DAEMON RUN] Failed to spawn process - no PID returned'); + if (codexHomeDirCleanup && !codexHomeDirCleanupArmed) { + codexHomeDirCleanup(); + codexHomeDirCleanup = null; + } return { type: 'error', errorMessage: 'Failed to spawn Happy process - no PID returned' @@ -536,6 +561,10 @@ export async function startDaemon(): Promise { }; pidToTrackedSession.set(happyProcess.pid, trackedSession); + if (codexHomeDirCleanup) { + codexHomeDirCleanupByPid.set(happyProcess.pid, codexHomeDirCleanup); + codexHomeDirCleanupArmed = true; + } happyProcess.on('exit', (code, signal) => { logger.debug(`[DAEMON RUN] Child PID ${happyProcess.pid} exited with code ${code}, signal ${signal}`); @@ -585,6 +614,10 @@ export async function startDaemon(): Promise { errorMessage: 'Unexpected error in session spawning' }; } catch (error) { + if (codexHomeDirCleanup && !codexHomeDirCleanupArmed) { + codexHomeDirCleanup(); + codexHomeDirCleanup = null; + } const errorMessage = error instanceof Error ? error.message : String(error); logger.debug('[DAEMON RUN] Failed to spawn session:', error); return { @@ -633,6 +666,15 @@ export async function startDaemon(): Promise { // Handle child process exit const onChildExited = (pid: number) => { logger.debug(`[DAEMON RUN] Removing exited process PID ${pid} from tracking`); + const cleanup = codexHomeDirCleanupByPid.get(pid); + if (cleanup) { + codexHomeDirCleanupByPid.delete(pid); + try { + cleanup(); + } catch (error) { + logger.debug('[DAEMON RUN] Failed to cleanup CODEX_HOME tmp dir', error); + } + } pidToTrackedSession.delete(pid); }; @@ -713,10 +755,34 @@ export async function startDaemon(): Promise { } catch (error) { // Process is dead, remove from tracking logger.debug(`[DAEMON RUN] Removing stale session with PID ${pid} (process no longer exists)`); + const cleanup = codexHomeDirCleanupByPid.get(pid); + if (cleanup) { + codexHomeDirCleanupByPid.delete(pid); + try { + cleanup(); + } catch (cleanupError) { + logger.debug('[DAEMON RUN] Failed to cleanup CODEX_HOME tmp dir', cleanupError); + } + } pidToTrackedSession.delete(pid); } } + // Cleanup any CODEX_HOME temp dirs for sessions no longer tracked (e.g. stopSession removed them). + for (const [pid, cleanup] of codexHomeDirCleanupByPid.entries()) { + if (pidToTrackedSession.has(pid)) continue; + try { + process.kill(pid, 0); + } catch { + codexHomeDirCleanupByPid.delete(pid); + try { + cleanup(); + } catch (cleanupError) { + logger.debug('[DAEMON RUN] Failed to cleanup CODEX_HOME tmp dir', cleanupError); + } + } + } + // Check if daemon needs update // If version on disk is different from the one in package.json - we need to restart // BIG if - does this get updated from underneath us on npm upgrade? diff --git a/src/modules/common/registerCommonHandlers.previewEnv.test.ts b/src/modules/common/registerCommonHandlers.previewEnv.test.ts new file mode 100644 index 00000000..70ebeb8f --- /dev/null +++ b/src/modules/common/registerCommonHandlers.previewEnv.test.ts @@ -0,0 +1,151 @@ +/** + * Tests for the `preview-env` RPC handler. + * + * Ensures the daemon can safely preview effective environment variable values + * (including ${VAR} expansion) without exposing secrets by default. + */ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { RpcHandlerManager } from '@/api/rpc/RpcHandlerManager'; +import type { RpcRequest } from '@/api/rpc/types'; +import { decodeBase64, decrypt, encodeBase64, encrypt } from '@/api/encryption'; +import { registerCommonHandlers } from './registerCommonHandlers'; + +function createTestRpcManager(params?: { scopePrefix?: string }) { + const encryptionKey = new Uint8Array(32).fill(7); + const encryptionVariant = 'legacy' as const; + const scopePrefix = params?.scopePrefix ?? 'machine-test'; + + const manager = new RpcHandlerManager({ + scopePrefix, + encryptionKey, + encryptionVariant, + logger: () => undefined, + }); + + registerCommonHandlers(manager, process.cwd()); + + async function call(method: string, request: TRequest): Promise { + const encryptedParams = encodeBase64(encrypt(encryptionKey, encryptionVariant, request)); + const rpcRequest: RpcRequest = { + method: `${scopePrefix}:${method}`, + params: encryptedParams, + }; + const encryptedResponse = await manager.handleRequest(rpcRequest); + const decrypted = decrypt(encryptionKey, encryptionVariant, decodeBase64(encryptedResponse)); + return decrypted as TResponse; + } + + return { call }; +} + +describe('registerCommonHandlers preview-env', () => { + const originalEnv = { ...process.env }; + + beforeEach(() => { + process.env = { ...originalEnv }; + }); + + afterEach(() => { + process.env = { ...originalEnv }; + }); + + it('returns effective env values with embedded ${VAR} expansion', async () => { + process.env.PATH = '/usr/bin'; + process.env.HAPPY_ENV_PREVIEW_SECRETS = 'none'; + + const { call } = createTestRpcManager(); + + const result = await call<{ policy: string; values: Record }, { + keys: string[]; + extraEnv?: Record; + }>('preview-env', { + keys: ['PATH'], + extraEnv: { + PATH: '/opt/bin:${PATH}', + }, + }); + + expect(result.policy).toBe('none'); + expect(result.values.PATH.display).toBe('full'); + expect(result.values.PATH.value).toBe('/opt/bin:/usr/bin'); + }); + + it('hides sensitive values when HAPPY_ENV_PREVIEW_SECRETS=none', async () => { + process.env.SECRET_TOKEN = 'sk-1234567890'; + process.env.HAPPY_ENV_PREVIEW_SECRETS = 'none'; + + const { call } = createTestRpcManager(); + + const result = await call<{ policy: string; values: Record }, { + keys: string[]; + extraEnv?: Record; + sensitiveHints?: Record; + }>('preview-env', { + keys: ['ANTHROPIC_AUTH_TOKEN'], + extraEnv: { + ANTHROPIC_AUTH_TOKEN: '${SECRET_TOKEN}', + }, + sensitiveHints: { + SECRET_TOKEN: true, + ANTHROPIC_AUTH_TOKEN: true, + }, + }); + + expect(result.policy).toBe('none'); + expect(result.values.ANTHROPIC_AUTH_TOKEN.isSensitive).toBe(true); + expect(result.values.ANTHROPIC_AUTH_TOKEN.display).toBe('hidden'); + expect(result.values.ANTHROPIC_AUTH_TOKEN.value).toBeNull(); + }); + + it('redacts sensitive values when HAPPY_ENV_PREVIEW_SECRETS=redacted', async () => { + process.env.SECRET_TOKEN = 'sk-1234567890'; + process.env.HAPPY_ENV_PREVIEW_SECRETS = 'redacted'; + + const { call } = createTestRpcManager(); + + const result = await call<{ policy: string; values: Record }, { + keys: string[]; + extraEnv?: Record; + sensitiveHints?: Record; + }>('preview-env', { + keys: ['ANTHROPIC_AUTH_TOKEN'], + extraEnv: { + ANTHROPIC_AUTH_TOKEN: '${SECRET_TOKEN}', + }, + sensitiveHints: { + SECRET_TOKEN: true, + ANTHROPIC_AUTH_TOKEN: true, + }, + }); + + expect(result.policy).toBe('redacted'); + expect(result.values.ANTHROPIC_AUTH_TOKEN.display).toBe('redacted'); + expect(result.values.ANTHROPIC_AUTH_TOKEN.value).toBe('sk-*******890'); + }); + + it('returns full sensitive values when HAPPY_ENV_PREVIEW_SECRETS=full', async () => { + process.env.SECRET_TOKEN = 'sk-1234567890'; + process.env.HAPPY_ENV_PREVIEW_SECRETS = 'full'; + + const { call } = createTestRpcManager(); + + const result = await call<{ policy: string; values: Record }, { + keys: string[]; + extraEnv?: Record; + sensitiveHints?: Record; + }>('preview-env', { + keys: ['ANTHROPIC_AUTH_TOKEN'], + extraEnv: { + ANTHROPIC_AUTH_TOKEN: '${SECRET_TOKEN}', + }, + sensitiveHints: { + SECRET_TOKEN: true, + ANTHROPIC_AUTH_TOKEN: true, + }, + }); + + expect(result.policy).toBe('full'); + expect(result.values.ANTHROPIC_AUTH_TOKEN.display).toBe('full'); + expect(result.values.ANTHROPIC_AUTH_TOKEN.value).toBe('sk-1234567890'); + }); +}); diff --git a/src/modules/common/registerCommonHandlers.ts b/src/modules/common/registerCommonHandlers.ts index bd4e07a5..c2fcab18 100644 --- a/src/modules/common/registerCommonHandlers.ts +++ b/src/modules/common/registerCommonHandlers.ts @@ -6,6 +6,7 @@ import { createHash } from 'crypto'; import { join } from 'path'; import { run as runRipgrep } from '@/modules/ripgrep/index'; import { run as runDifftastic } from '@/modules/difftastic/index'; +import { expandEnvironmentVariables } from '@/utils/expandEnvVars'; import { RpcHandlerManager } from '../../api/rpc/RpcHandlerManager'; import { validatePath } from './pathSecurity'; @@ -25,6 +26,26 @@ interface BashResponse { error?: string; } +type EnvPreviewSecretsPolicy = 'none' | 'redacted' | 'full'; + +interface PreviewEnvRequest { + keys: string[]; + extraEnv?: Record; + sensitiveHints?: Record; +} + +interface PreviewEnvValue { + value: string | null; + isSet: boolean; + isSensitive: boolean; + display: 'full' | 'redacted' | 'hidden' | 'unset'; +} + +interface PreviewEnvResponse { + policy: EnvPreviewSecretsPolicy; + values: Record; +} + interface ReadFileRequest { path: string; } @@ -122,19 +143,25 @@ export interface SpawnSessionOptions { approvedNewDirectoryCreation?: boolean; agent?: 'claude' | 'codex' | 'gemini'; token?: string; - environmentVariables?: { - // Anthropic Claude API configuration - ANTHROPIC_BASE_URL?: string; // Custom API endpoint (overrides default) - ANTHROPIC_AUTH_TOKEN?: string; // API authentication token - ANTHROPIC_MODEL?: string; // Model to use (e.g., claude-3-5-sonnet-20241022) - - // Tmux session management environment variables - // Based on tmux(1) manual and common tmux usage patterns - TMUX_SESSION_NAME?: string; // Name for tmux session (creates/attaches to named session) - TMUX_TMPDIR?: string; // Temporary directory for tmux server socket files - // Note: TMUX_TMPDIR is used by tmux to store socket files when default /tmp is not suitable - // Common use case: When /tmp has limited space or different permissions - }; + /** + * Session-scoped profile identity for display/debugging across devices. + * This is NOT the profile content; actual runtime behavior is still driven + * by environmentVariables passed for this spawn. + * + * Empty string is allowed and means "no profile". + */ + profileId?: string; + /** + * Arbitrary environment variables for the spawned session. + * + * The GUI builds these from a profile (env var list + tmux settings) and may include + * provider-specific keys like: + * - ANTHROPIC_AUTH_TOKEN / ANTHROPIC_BASE_URL / ANTHROPIC_MODEL + * - OPENAI_API_KEY / OPENAI_BASE_URL / OPENAI_MODEL + * - AZURE_OPENAI_* / TOGETHER_* + * - TMUX_SESSION_NAME / TMUX_TMPDIR + */ + environmentVariables?: Record; } export type SpawnSessionResult = @@ -147,6 +174,48 @@ export type SpawnSessionResult = */ export function registerCommonHandlers(rpcHandlerManager: RpcHandlerManager, workingDirectory: string) { + function normalizeSecretsPolicy(raw: unknown): EnvPreviewSecretsPolicy { + if (typeof raw !== 'string') return 'none'; + const normalized = raw.trim().toLowerCase(); + if (normalized === 'none' || normalized === 'redacted' || normalized === 'full') return normalized; + return 'none'; + } + + function clampInt(value: number, min: number, max: number): number { + if (!Number.isFinite(value)) return min; + return Math.max(min, Math.min(max, Math.trunc(value))); + } + + function redactSecret(value: string): string { + const len = value.length; + if (len <= 0) return ''; + if (len <= 2) return '*'.repeat(len); + + // Hybrid: percentage with min/max caps (credit-card style). + const ratio = 0.2; + const startRaw = Math.ceil(len * ratio); + const endRaw = Math.ceil(len * ratio); + + let start = clampInt(startRaw, 1, 6); + let end = clampInt(endRaw, 1, 6); + + // Ensure we always have at least 1 masked character (when possible). + if (start + end >= len) { + // Keep start/end small enough to leave room for masking. + // Prefer preserving start, then reduce end. + end = Math.max(0, len - start - 1); + if (end < 1) { + start = Math.max(0, len - 2); + end = Math.max(0, len - start - 1); + } + } + + const maskedLen = Math.max(0, len - start - end); + const prefix = value.slice(0, start); + const suffix = end > 0 ? value.slice(len - end) : ''; + return `${prefix}${'*'.repeat(maskedLen)}${suffix}`; + } + // Shell command handler - executes commands in the default shell rpcHandlerManager.registerHandler('bash', async (data) => { logger.debug('Shell command request:', data.command); @@ -231,6 +300,73 @@ export function registerCommonHandlers(rpcHandlerManager: RpcHandlerManager, wor } }); + // Environment preview handler - returns daemon-effective env values with secret policy applied. + // + // This is the recommended way for the UI to preview what a spawned session will receive: + // - Uses daemon process.env as the base + // - Optionally applies profile-provided extraEnv with the same ${VAR} expansion semantics used for spawns + // - Applies daemon-controlled secret visibility policy (HAPPY_ENV_PREVIEW_SECRETS) + rpcHandlerManager.registerHandler('preview-env', async (data) => { + const keys = Array.isArray(data?.keys) ? data.keys : []; + const maxKeys = 200; + const trimmedKeys = keys.slice(0, maxKeys); + + const validNameRegex = /^[A-Z_][A-Z0-9_]*$/; + for (const key of trimmedKeys) { + if (typeof key !== 'string' || !validNameRegex.test(key)) { + throw new Error(`Invalid env var key: "${String(key)}"`); + } + } + + const policy = normalizeSecretsPolicy(process.env.HAPPY_ENV_PREVIEW_SECRETS); + const sensitiveHints = data?.sensitiveHints && typeof data.sensitiveHints === 'object' + ? data.sensitiveHints + : {}; + + const extraEnvRaw = data?.extraEnv && typeof data.extraEnv === 'object' ? data.extraEnv : {}; + const extraEnv: Record = {}; + for (const [k, v] of Object.entries(extraEnvRaw)) { + if (typeof k !== 'string' || !validNameRegex.test(k)) continue; + if (typeof v !== 'string') continue; + extraEnv[k] = v; + } + + const expandedExtraEnv = Object.keys(extraEnv).length > 0 + ? expandEnvironmentVariables(extraEnv, process.env, { warnOnUndefined: false }) + : {}; + const effectiveEnv: NodeJS.ProcessEnv = { ...process.env, ...expandedExtraEnv }; + + const SECRET_NAME_REGEX = /TOKEN|KEY|SECRET|AUTH|PASS|PASSWORD|COOKIE/i; + + const values: Record = {}; + for (const key of trimmedKeys) { + const rawValue = effectiveEnv[key]; + const isSet = typeof rawValue === 'string'; + const hintedSensitive = (sensitiveHints as Record)[key] === true; + const isSensitive = hintedSensitive || SECRET_NAME_REGEX.test(key); + + if (!isSet) { + values[key] = { value: null, isSet: false, isSensitive, display: 'unset' }; + continue; + } + + if (!isSensitive) { + values[key] = { value: rawValue, isSet: true, isSensitive: false, display: 'full' }; + continue; + } + + if (policy === 'none') { + values[key] = { value: null, isSet: true, isSensitive: true, display: 'hidden' }; + } else if (policy === 'redacted') { + values[key] = { value: redactSecret(rawValue), isSet: true, isSensitive: true, display: 'redacted' }; + } else { + values[key] = { value: rawValue, isSet: true, isSensitive: true, display: 'full' }; + } + } + + return { policy, values }; + }); + // Read file handler - returns base64 encoded content rpcHandlerManager.registerHandler('readFile', async (data) => { logger.debug('Read file request:', data.path); @@ -519,4 +655,4 @@ export function registerCommonHandlers(rpcHandlerManager: RpcHandlerManager, wor }; } }); -} \ No newline at end of file +} diff --git a/src/persistence.profileSchema.test.ts b/src/persistence.profileSchema.test.ts new file mode 100644 index 00000000..5b231138 --- /dev/null +++ b/src/persistence.profileSchema.test.ts @@ -0,0 +1,58 @@ +import { describe, expect, it } from 'vitest'; +import { AIBackendProfileSchema } from './persistence'; + +describe('AIBackendProfileSchema legacy provider config migration', () => { + it('migrates legacy provider config objects into environmentVariables', () => { + const profile = AIBackendProfileSchema.parse({ + id: 'profile-1', + name: 'Profile 1', + openaiConfig: { + apiKey: '${OPENAI_KEY}', + }, + }); + + expect(profile.environmentVariables).toContainEqual({ name: 'OPENAI_API_KEY', value: '${OPENAI_KEY}' }); + expect((profile as any).openaiConfig).toBeUndefined(); + }); + + it('migrates other legacy provider config objects into environmentVariables', () => { + const profile = AIBackendProfileSchema.parse({ + id: 'profile-1', + name: 'Profile 1', + anthropicConfig: { + authToken: '${ANTHROPIC_KEY}', + baseUrl: '${ANTHROPIC_URL}', + }, + azureOpenAIConfig: { + apiKey: '${AZURE_KEY}', + endpoint: '${AZURE_ENDPOINT}', + deploymentName: '${AZURE_DEPLOYMENT}', + }, + togetherAIConfig: { + apiKey: '${TOGETHER_KEY}', + }, + }); + + expect(profile.environmentVariables).toContainEqual({ name: 'ANTHROPIC_AUTH_TOKEN', value: '${ANTHROPIC_KEY}' }); + expect(profile.environmentVariables).toContainEqual({ name: 'ANTHROPIC_BASE_URL', value: '${ANTHROPIC_URL}' }); + expect(profile.environmentVariables).toContainEqual({ name: 'AZURE_OPENAI_API_KEY', value: '${AZURE_KEY}' }); + expect(profile.environmentVariables).toContainEqual({ name: 'AZURE_OPENAI_ENDPOINT', value: '${AZURE_ENDPOINT}' }); + expect(profile.environmentVariables).toContainEqual({ name: 'AZURE_OPENAI_DEPLOYMENT_NAME', value: '${AZURE_DEPLOYMENT}' }); + expect(profile.environmentVariables).toContainEqual({ name: 'TOGETHER_API_KEY', value: '${TOGETHER_KEY}' }); + }); + + it('does not override explicit environmentVariables with legacy config values', () => { + const profile = AIBackendProfileSchema.parse({ + id: 'profile-1', + name: 'Profile 1', + environmentVariables: [{ name: 'OPENAI_API_KEY', value: 'explicit' }], + openaiConfig: { + apiKey: 'legacy', + }, + }); + + const apiKeyEntries = profile.environmentVariables.filter((ev) => ev.name === 'OPENAI_API_KEY'); + expect(apiKeyEntries).toHaveLength(1); + expect(apiKeyEntries[0]?.value).toBe('explicit'); + }); +}); diff --git a/src/persistence.ts b/src/persistence.ts index 0270aaf5..d36a7c93 100644 --- a/src/persistence.ts +++ b/src/persistence.ts @@ -16,36 +16,88 @@ import { logger } from '@/ui/logger'; // AI backend profile schema - MUST match happy app exactly // Using same Zod schema as GUI for runtime validation consistency -// Environment variable schemas for different AI providers (matching GUI exactly) -const AnthropicConfigSchema = z.object({ - baseUrl: z.string().url().optional(), - authToken: z.string().optional(), - model: z.string().optional(), -}); +function mergeEnvironmentVariables( + existing: unknown, + additions: Record +): Array<{ name: string; value: string }> { + /** + * Merge strategy: preserve explicit `environmentVariables` entries. + * + * Legacy provider config objects (e.g. `openaiConfig.apiKey`) are treated as + * "defaults" and only fill missing keys, so they never override a user-set + * env var entry that already exists in `environmentVariables`. + */ + const map = new Map(); + + if (Array.isArray(existing)) { + for (const entry of existing) { + if (!entry || typeof entry !== 'object') continue; + const record = entry as Record; + const name = record.name; + const value = record.value; + if (typeof name !== 'string' || typeof value !== 'string') continue; + map.set(name, value); + } + } -const OpenAIConfigSchema = z.object({ - apiKey: z.string().optional(), - baseUrl: z.string().url().optional(), - model: z.string().optional(), -}); + for (const [name, value] of Object.entries(additions)) { + if (typeof value !== 'string') continue; + if (!map.has(name)) { + map.set(name, value); + } + } -const AzureOpenAIConfigSchema = z.object({ - apiKey: z.string().optional(), - endpoint: z.string().url().optional(), - apiVersion: z.string().optional(), - deploymentName: z.string().optional(), -}); + return Array.from(map.entries()).map(([name, value]) => ({ name, value })); +} -const TogetherAIConfigSchema = z.object({ - apiKey: z.string().optional(), - model: z.string().optional(), -}); +function normalizeLegacyProfileConfig(profile: unknown): unknown { + if (!profile || typeof profile !== 'object') return profile; + + const raw = profile as Record; + + const readString = (value: unknown): string | undefined => (typeof value === 'string' ? value : undefined); + const asRecord = (value: unknown): Record | null => + value && typeof value === 'object' && !Array.isArray(value) ? (value as Record) : null; + + const anthropicConfig = asRecord(raw.anthropicConfig); + const openaiConfig = asRecord(raw.openaiConfig); + const azureOpenAIConfig = asRecord(raw.azureOpenAIConfig); + const togetherAIConfig = asRecord(raw.togetherAIConfig); + + const additions: Record = { + ANTHROPIC_BASE_URL: readString(anthropicConfig?.baseUrl), + ANTHROPIC_AUTH_TOKEN: readString(anthropicConfig?.authToken), + ANTHROPIC_MODEL: readString(anthropicConfig?.model), + OPENAI_API_KEY: readString(openaiConfig?.apiKey), + OPENAI_BASE_URL: readString(openaiConfig?.baseUrl), + OPENAI_MODEL: readString(openaiConfig?.model), + AZURE_OPENAI_API_KEY: readString(azureOpenAIConfig?.apiKey), + AZURE_OPENAI_ENDPOINT: readString(azureOpenAIConfig?.endpoint), + AZURE_OPENAI_API_VERSION: readString(azureOpenAIConfig?.apiVersion), + AZURE_OPENAI_DEPLOYMENT_NAME: readString(azureOpenAIConfig?.deploymentName), + TOGETHER_API_KEY: readString(togetherAIConfig?.apiKey), + TOGETHER_MODEL: readString(togetherAIConfig?.model), + }; + + const environmentVariables = mergeEnvironmentVariables(raw.environmentVariables, additions); + + // Remove legacy provider config objects. Any values are preserved via environmentVariables migration above. + const rest: Record = { ...raw }; + delete rest.anthropicConfig; + delete rest.openaiConfig; + delete rest.azureOpenAIConfig; + delete rest.togetherAIConfig; + + return { + ...rest, + environmentVariables, + }; +} // Tmux configuration schema (matching GUI exactly) const TmuxConfigSchema = z.object({ sessionName: z.string().optional(), tmpDir: z.string().optional(), - updateEnvironment: z.boolean().optional(), }); // Environment variables schema with validation (matching GUI exactly) @@ -61,18 +113,13 @@ const ProfileCompatibilitySchema = z.object({ gemini: z.boolean().default(true), }); -// AIBackendProfile schema - EXACT MATCH with GUI schema -export const AIBackendProfileSchema = z.object({ - id: z.string().uuid(), +// AIBackendProfile schema - MUST match happy app +export const AIBackendProfileSchema = z.preprocess(normalizeLegacyProfileConfig, z.object({ + // Accept both UUIDs (user profiles) and simple strings (built-in profiles) + id: z.string().min(1), name: z.string().min(1).max(100), description: z.string().max(500).optional(), - // Agent-specific configurations - anthropicConfig: AnthropicConfigSchema.optional(), - openaiConfig: OpenAIConfigSchema.optional(), - azureOpenAIConfig: AzureOpenAIConfigSchema.optional(), - togetherAIConfig: TogetherAIConfigSchema.optional(), - // Tmux configuration tmuxConfig: TmuxConfigSchema.optional(), @@ -101,7 +148,7 @@ export const AIBackendProfileSchema = z.object({ createdAt: z.number().default(() => Date.now()), updatedAt: z.number().default(() => Date.now()), version: z.string().default('1.0.0'), -}); +})); export type AIBackendProfile = z.infer; @@ -118,42 +165,12 @@ export function getProfileEnvironmentVariables(profile: AIBackendProfile): Recor envVars[envVar.name] = envVar.value; }); - // Add Anthropic config - if (profile.anthropicConfig) { - if (profile.anthropicConfig.baseUrl) envVars.ANTHROPIC_BASE_URL = profile.anthropicConfig.baseUrl; - if (profile.anthropicConfig.authToken) envVars.ANTHROPIC_AUTH_TOKEN = profile.anthropicConfig.authToken; - if (profile.anthropicConfig.model) envVars.ANTHROPIC_MODEL = profile.anthropicConfig.model; - } - - // Add OpenAI config - if (profile.openaiConfig) { - if (profile.openaiConfig.apiKey) envVars.OPENAI_API_KEY = profile.openaiConfig.apiKey; - if (profile.openaiConfig.baseUrl) envVars.OPENAI_BASE_URL = profile.openaiConfig.baseUrl; - if (profile.openaiConfig.model) envVars.OPENAI_MODEL = profile.openaiConfig.model; - } - - // Add Azure OpenAI config - if (profile.azureOpenAIConfig) { - if (profile.azureOpenAIConfig.apiKey) envVars.AZURE_OPENAI_API_KEY = profile.azureOpenAIConfig.apiKey; - if (profile.azureOpenAIConfig.endpoint) envVars.AZURE_OPENAI_ENDPOINT = profile.azureOpenAIConfig.endpoint; - if (profile.azureOpenAIConfig.apiVersion) envVars.AZURE_OPENAI_API_VERSION = profile.azureOpenAIConfig.apiVersion; - if (profile.azureOpenAIConfig.deploymentName) envVars.AZURE_OPENAI_DEPLOYMENT_NAME = profile.azureOpenAIConfig.deploymentName; - } - - // Add Together AI config - if (profile.togetherAIConfig) { - if (profile.togetherAIConfig.apiKey) envVars.TOGETHER_API_KEY = profile.togetherAIConfig.apiKey; - if (profile.togetherAIConfig.model) envVars.TOGETHER_MODEL = profile.togetherAIConfig.model; - } - // Add Tmux config if (profile.tmuxConfig) { // Empty string means "use current/most recent session", so include it if (profile.tmuxConfig.sessionName !== undefined) envVars.TMUX_SESSION_NAME = profile.tmuxConfig.sessionName; - if (profile.tmuxConfig.tmpDir) envVars.TMUX_TMPDIR = profile.tmuxConfig.tmpDir; - if (profile.tmuxConfig.updateEnvironment !== undefined) { - envVars.TMUX_UPDATE_ENVIRONMENT = profile.tmuxConfig.updateEnvironment.toString(); - } + // Empty string may be valid to use tmux defaults; include if explicitly provided. + if (profile.tmuxConfig.tmpDir !== undefined) envVars.TMUX_TMPDIR = profile.tmuxConfig.tmpDir; } return envVars; @@ -177,7 +194,8 @@ export const CURRENT_PROFILE_VERSION = '1.0.0'; // Settings schema version: Integer for overall Settings structure compatibility // Incremented when Settings structure changes (e.g., adding profiles array was v1→v2) // Used for migration logic in readSettings() -export const SUPPORTED_SCHEMA_VERSION = 2; +// NOTE: This is the schema for happy-cli's local settings file (not the Happy app's server-synced account settings). +export const SUPPORTED_SCHEMA_VERSION = 3; // Profile version validation export function validateProfileVersion(profile: AIBackendProfile): boolean { @@ -206,15 +224,12 @@ interface Settings { // Profile management settings (synced with happy app) activeProfileId?: string profiles: AIBackendProfile[] - // CLI-local environment variable cache (not synced) - localEnvironmentVariables: Record> // profileId -> env vars } const defaultSettings: Settings = { schemaVersion: SUPPORTED_SCHEMA_VERSION, onboardingCompleted: false, profiles: [], - localEnvironmentVariables: {} } /** @@ -230,14 +245,18 @@ function migrateSettings(raw: any, fromVersion: number): any { if (!migrated.profiles) { migrated.profiles = []; } - // Ensure localEnvironmentVariables exists - if (!migrated.localEnvironmentVariables) { - migrated.localEnvironmentVariables = {}; - } // Update schema version migrated.schemaVersion = 2; } + // Migration from v2 to v3 (removed CLI-local env cache) + if (fromVersion < 3) { + if ('localEnvironmentVariables' in migrated) { + delete migrated.localEnvironmentVariables; + } + migrated.schemaVersion = 3; + } + // Future migrations go here: // if (fromVersion < 3) { ... } @@ -641,66 +660,3 @@ export async function updateProfiles(profiles: unknown[]): Promise { }; }); } - -/** - * Get environment variables for a profile - * Combines profile custom env vars with CLI-local cached env vars - */ -export async function getEnvironmentVariables(profileId: string): Promise> { - const settings = await readSettings(); - const profile = settings.profiles.find(p => p.id === profileId); - if (!profile) return {}; - - // Start with profile's environment variables (new schema) - const envVars: Record = {}; - if (profile.environmentVariables) { - profile.environmentVariables.forEach(envVar => { - envVars[envVar.name] = envVar.value; - }); - } - - // Override with CLI-local cached environment variables - const localEnvVars = settings.localEnvironmentVariables[profileId] || {}; - Object.assign(envVars, localEnvVars); - - return envVars; -} - -/** - * Set environment variables for a profile in CLI-local cache - */ -export async function setEnvironmentVariables(profileId: string, envVars: Record): Promise { - await updateSettings(settings => ({ - ...settings, - localEnvironmentVariables: { - ...settings.localEnvironmentVariables, - [profileId]: envVars - } - })); -} - -/** - * Get a specific environment variable for a profile - * Checks CLI-local cache first, then profile environment variables - */ -export async function getEnvironmentVariable(profileId: string, key: string): Promise { - const settings = await readSettings(); - - // Check CLI-local cache first - const localEnvVars = settings.localEnvironmentVariables[profileId] || {}; - if (localEnvVars[key] !== undefined) { - return localEnvVars[key]; - } - - // Fall back to profile environment variables (new schema) - const profile = settings.profiles.find(p => p.id === profileId); - if (profile?.environmentVariables) { - const envVar = profile.environmentVariables.find(env => env.name === key); - if (envVar) { - return envVar.value; - } - } - - return undefined; -} - diff --git a/src/ui/doctor.test.ts b/src/ui/doctor.test.ts new file mode 100644 index 00000000..14f93d82 --- /dev/null +++ b/src/ui/doctor.test.ts @@ -0,0 +1,16 @@ +import { describe, it, expect } from 'vitest'; +import { maskValue } from './doctor'; + +describe('doctor redaction', () => { + it('does not treat ${VAR:-default} templates as safe', () => { + expect(maskValue('${SAFE_TEMPLATE}')).toBe('${SAFE_TEMPLATE}'); + expect(maskValue('${LEAK:-sk-live-secret}')).toMatch(/^\$\{LEAK:-<\d+ chars>\}$/); + expect(maskValue('${LEAK:=sk-live-secret}')).toMatch(/^\$\{LEAK:=<\d+ chars>\}$/); + }); + + it('handles empty, undefined, and plain secret values', () => { + expect(maskValue('')).toBe(''); + expect(maskValue(undefined)).toBeUndefined(); + expect(maskValue('sk-live-secret')).toBe('<14 chars>'); + }); +}); diff --git a/src/ui/doctor.ts b/src/ui/doctor.ts index 08477498..4c9908e1 100644 --- a/src/ui/doctor.ts +++ b/src/ui/doctor.ts @@ -17,6 +17,56 @@ import { join } from 'node:path' import { projectPath } from '@/projectPath' import packageJson from '../../package.json' +export function maskValue(value: string): string; +export function maskValue(value: string | undefined): string | undefined; +export function maskValue(value: string | undefined): string | undefined { + if (value === undefined) return undefined; + if (value.trim() === '') return ''; + + // Treat ${VAR} templates as safe to display (they do not contain secrets themselves). + if (/^\$\{[A-Z_][A-Z0-9_]*\}$/.test(value)) return value; + + // For templates with default values, preserve the template structure but mask the fallback. + // Example: ${OPENAI_API_KEY:-sk-...} -> ${OPENAI_API_KEY:-} + const matchWithFallback = value.match(/^\$\{([A-Z_][A-Z0-9_]*)(:-|:=)(.*)\}$/); + if (matchWithFallback) { + const [, sourceVar, operator, fallback] = matchWithFallback; + if (fallback === '') return `\${${sourceVar}${operator}}`; + return `\${${sourceVar}${operator}${maskValue(fallback)}}`; + } + + return `<${value.length} chars>`; +} + +type SettingsForDisplay = Awaited>; + +function redactSettingsForDisplay(settings: SettingsForDisplay): SettingsForDisplay { + const redacted = JSON.parse(JSON.stringify(settings ?? {})) as SettingsForDisplay; + const redactedRecord = redacted as unknown as Record; + + // Remove any legacy CLI-local env cache; it may contain secrets. + if (Object.prototype.hasOwnProperty.call(redactedRecord, 'localEnvironmentVariables')) { + delete redactedRecord.localEnvironmentVariables; + } + + if (Array.isArray(redacted.profiles)) { + redacted.profiles = redacted.profiles.map((profile) => { + const p = { ...profile }; + + if (Array.isArray(p.environmentVariables)) { + p.environmentVariables = p.environmentVariables.map((ev) => ({ + ...ev, + value: maskValue(ev.value), + })); + } + + return p; + }); + } + + return redacted; +} + /** * Get relevant environment information for debugging */ @@ -120,7 +170,7 @@ export async function runDoctorCommand(filter?: 'all' | 'daemon'): Promise try { const settings = await readSettings(); console.log(chalk.bold('\n📄 Settings (settings.json):')); - console.log(chalk.gray(JSON.stringify(settings, null, 2))); + console.log(chalk.gray(JSON.stringify(redactSettingsForDisplay(settings), null, 2))); } catch (error) { console.log(chalk.bold('\n📄 Settings:')); console.log(chalk.red('❌ Failed to read settings')); @@ -266,4 +316,4 @@ export async function runDoctorCommand(filter?: 'all' | 'daemon'): Promise } console.log(chalk.green('\n✅ Doctor diagnosis complete!\n')); -} \ No newline at end of file +} diff --git a/src/ui/logger.test.ts b/src/ui/logger.test.ts new file mode 100644 index 00000000..3fc89160 --- /dev/null +++ b/src/ui/logger.test.ts @@ -0,0 +1,46 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { existsSync, mkdtempSync, readFileSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; + +describe('logger.debugLargeJson', () => { + const originalDebug = process.env.DEBUG; + const originalHappyHomeDir = process.env.HAPPY_HOME_DIR; + let tempDir: string; + + beforeEach(() => { + tempDir = mkdtempSync(join(tmpdir(), 'happy-cli-logger-test-')); + process.env.HAPPY_HOME_DIR = tempDir; + delete process.env.DEBUG; + vi.resetModules(); + }); + + afterEach(() => { + rmSync(tempDir, { recursive: true, force: true }); + if (originalHappyHomeDir === undefined) delete process.env.HAPPY_HOME_DIR; + else process.env.HAPPY_HOME_DIR = originalHappyHomeDir; + + if (originalDebug === undefined) delete process.env.DEBUG; + else process.env.DEBUG = originalDebug; + }); + + it('does not write to log file when DEBUG is not set', async () => { + const { logger } = (await import('@/ui/logger')) as typeof import('@/ui/logger'); + + logger.debugLargeJson('[TEST] debugLargeJson', { secret: 'value' }); + + expect(existsSync(logger.getLogPath())).toBe(false); + }); + + it('writes to log file when DEBUG is set', async () => { + const { logger } = (await import('@/ui/logger')) as typeof import('@/ui/logger'); + process.env.DEBUG = '1'; + + logger.debugLargeJson('[TEST] debugLargeJson', { secret: 'value' }); + + expect(existsSync(logger.getLogPath())).toBe(true); + const content = readFileSync(logger.getLogPath(), 'utf8'); + expect(content).toContain('[TEST] debugLargeJson'); + }); +}); + diff --git a/src/ui/logger.ts b/src/ui/logger.ts index ecf739b6..1a8a5b0d 100644 --- a/src/ui/logger.ts +++ b/src/ui/logger.ts @@ -84,9 +84,7 @@ class Logger { maxStringLength: number = 100, maxArrayLength: number = 10, ): void { - if (!process.env.DEBUG) { - this.debug(`In production, skipping message inspection`) - } + if (!process.env.DEBUG) return; // Some of our messages are huge, but we still want to show them in the logs const truncateStrings = (obj: unknown): unknown => { diff --git a/src/utils/createSessionMetadata.ts b/src/utils/createSessionMetadata.ts index 4511b0c8..84e85be0 100644 --- a/src/utils/createSessionMetadata.ts +++ b/src/utils/createSessionMetadata.ts @@ -67,11 +67,15 @@ export function createSessionMetadata(opts: CreateSessionMetadataOptions): Sessi controlledByUser: false, }; + const profileIdEnv = process.env.HAPPY_SESSION_PROFILE_ID; + const profileId = profileIdEnv === undefined ? undefined : (profileIdEnv.trim() || null); + const metadata: Metadata = { path: process.cwd(), host: os.hostname(), version: packageJson.version, os: os.platform(), + ...(profileIdEnv !== undefined ? { profileId } : {}), machineId: opts.machineId, homeDir: os.homedir(), happyHomeDir: configuration.happyHomeDir, diff --git a/src/utils/expandEnvVars.test.ts b/src/utils/expandEnvVars.test.ts index bf9c9a02..c4f558cc 100644 --- a/src/utils/expandEnvVars.test.ts +++ b/src/utils/expandEnvVars.test.ts @@ -134,6 +134,70 @@ describe('expandEnvironmentVariables', () => { }); }); + it('should use default for ${VAR:-default} when VAR is missing', () => { + const envVars = { + TARGET: '${MISSING_VAR:-default-value}', + }; + const sourceEnv = {}; + + const result = expandEnvironmentVariables(envVars, sourceEnv); + expect(result).toEqual({ + TARGET: 'default-value', + }); + }); + + it('should use default for ${VAR:=default} when VAR is missing', () => { + const envVars = { + TARGET: '${MISSING_VAR:=default-value}', + }; + const sourceEnv = {}; + + const result = expandEnvironmentVariables(envVars, sourceEnv); + expect(result).toEqual({ + TARGET: 'default-value', + }); + }); + + it('reuses ${VAR:=default} assignments for subsequent references in the same value', () => { + const envVars = { + TARGET: '${MISSING_VAR:=default-value}-${MISSING_VAR}', + }; + const sourceEnv = {}; + + const result = expandEnvironmentVariables(envVars, sourceEnv); + expect(result).toEqual({ + TARGET: 'default-value-default-value', + }); + }); + + it('treats empty string as missing for ${VAR:-default}', () => { + const envVars = { + TARGET: '${EMPTY_VAR:-default-value}', + }; + const sourceEnv = { + EMPTY_VAR: '', + }; + + const result = expandEnvironmentVariables(envVars, sourceEnv); + expect(result).toEqual({ + TARGET: 'default-value', + }); + }); + + it('treats empty string as missing for ${VAR:=default}', () => { + const envVars = { + TARGET: '${EMPTY_VAR:=default-value}', + }; + const sourceEnv = { + EMPTY_VAR: '', + }; + + const result = expandEnvironmentVariables(envVars, sourceEnv); + expect(result).toEqual({ + TARGET: 'default-value', + }); + }); + it('should handle multiple variables with same source', () => { const envVars = { VAR1: '${SHARED}', diff --git a/src/utils/expandEnvVars.ts b/src/utils/expandEnvVars.ts index f4e08f77..1bd29532 100644 --- a/src/utils/expandEnvVars.ts +++ b/src/utils/expandEnvVars.ts @@ -8,8 +8,8 @@ import { logger } from '@/ui/logger'; * Example: { ANTHROPIC_AUTH_TOKEN: "${Z_AI_AUTH_TOKEN}" } * * When daemon spawns sessions: - * - Tmux mode: Shell automatically expands ${VAR} - * - Non-tmux mode: Node.js spawn does NOT expand ${VAR} + * - Tmux mode: tmux launches a shell, but shells do not expand ${VAR} placeholders embedded inside env values automatically + * - Non-tmux mode: Node.js spawn does NOT expand ${VAR} placeholders * * This utility ensures ${VAR} expansion works in both modes. * @@ -28,50 +28,70 @@ import { logger } from '@/ui/logger'; */ export function expandEnvironmentVariables( envVars: Record, - sourceEnv: NodeJS.ProcessEnv = process.env + sourceEnv: NodeJS.ProcessEnv = process.env, + options?: { + warnOnUndefined?: boolean; + } ): Record { const expanded: Record = {}; const undefinedVars: string[] = []; + const assignedEnv: Record = {}; + + function readEnv(varName: string): string | undefined { + if (Object.prototype.hasOwnProperty.call(assignedEnv, varName)) { + return assignedEnv[varName]; + } + return sourceEnv[varName]; + } for (const [key, value] of Object.entries(envVars)) { - // Replace all ${VAR} and ${VAR:-default} references with actual values from sourceEnv + // Replace all ${VAR}, ${VAR:-default}, and ${VAR:=default} references with actual values from sourceEnv const expandedValue = value.replace(/\$\{([^}]+)\}/g, (match, expr) => { - // Support bash parameter expansion: ${VAR:-default} + // Support bash parameter expansion: ${VAR:-default} and ${VAR:=default} // Example: ${Z_AI_BASE_URL:-https://api.z.ai/api/anthropic} const colonDashIndex = expr.indexOf(':-'); + const colonEqIndex = expr.indexOf(':='); let varName: string; let defaultValue: string | undefined; + let operator: ':-' | ':=' | null = null; - if (colonDashIndex !== -1) { - // Split ${VAR:-default} into varName and defaultValue - varName = expr.substring(0, colonDashIndex); - defaultValue = expr.substring(colonDashIndex + 2); + if (colonDashIndex !== -1 || colonEqIndex !== -1) { + // Split ${VAR:-default} or ${VAR:=default} into varName and defaultValue + const idx = colonDashIndex !== -1 && (colonEqIndex === -1 || colonDashIndex < colonEqIndex) + ? colonDashIndex + : colonEqIndex; + operator = idx === colonDashIndex ? ':-' : ':='; + varName = expr.substring(0, idx); + defaultValue = expr.substring(idx + 2); } else { // Simple ${VAR} reference varName = expr; } - const resolvedValue = sourceEnv[varName]; - if (resolvedValue !== undefined) { + const resolvedValue = readEnv(varName); + const shouldTreatEmptyAsMissing = defaultValue !== undefined; + const isMissing = resolvedValue === undefined || (shouldTreatEmptyAsMissing && resolvedValue === ''); + + if (!isMissing) { // Variable found in source environment - use its value - // Log for debugging (mask secret-looking values) - const isSensitive = varName.toLowerCase().includes('token') || - varName.toLowerCase().includes('key') || - varName.toLowerCase().includes('secret'); - const displayValue = isSensitive - ? (resolvedValue ? `<${resolvedValue.length} chars>` : '') - : resolvedValue; - logger.debug(`[EXPAND ENV] Expanded ${varName} from daemon env: ${displayValue}`); + if (process.env.DEBUG) { + logger.debug(`[EXPAND ENV] Expanded ${varName} from daemon env`); + } // Warn if empty string (common mistake) - if (resolvedValue === '') { + if (resolvedValue === '' && !Object.prototype.hasOwnProperty.call(assignedEnv, varName)) { logger.warn(`[EXPAND ENV] WARNING: ${varName} is set but EMPTY in daemon environment`); } return resolvedValue; } else if (defaultValue !== undefined) { // Variable not found but default value provided - use default - logger.debug(`[EXPAND ENV] Using default value for ${varName}: ${defaultValue}`); + if (process.env.DEBUG) { + logger.debug(`[EXPAND ENV] Using default value for ${varName}`); + } + if (operator === ':=') { + assignedEnv[varName] = defaultValue; + } return defaultValue; } else { // Variable not found and no default - keep placeholder and warn @@ -84,7 +104,8 @@ export function expandEnvironmentVariables( } // Log warning if any variables couldn't be resolved - if (undefinedVars.length > 0) { + const warnOnUndefined = options?.warnOnUndefined ?? true; + if (warnOnUndefined && undefinedVars.length > 0) { logger.warn(`[EXPAND ENV] Undefined variables referenced in profile environment: ${undefinedVars.join(', ')}`); logger.warn(`[EXPAND ENV] Session may fail to authenticate. Set these in daemon environment before launching:`); undefinedVars.forEach(varName => { diff --git a/src/utils/offlineSessionStub.test.ts b/src/utils/offlineSessionStub.test.ts new file mode 100644 index 00000000..9d35d1b2 --- /dev/null +++ b/src/utils/offlineSessionStub.test.ts @@ -0,0 +1,16 @@ +import { describe, expect, it } from 'vitest'; +import { createOfflineSessionStub } from '@/utils/offlineSessionStub'; + +describe('createOfflineSessionStub', () => { + it('returns an EventEmitter-compatible ApiSessionClient', () => { + const session = createOfflineSessionStub('tag'); + + let calls = 0; + session.on('message', () => { + calls += 1; + }); + session.emit('message', { ok: true }); + + expect(calls).toBe(1); + }); +}); diff --git a/src/utils/offlineSessionStub.ts b/src/utils/offlineSessionStub.ts index 50d67ab6..7bf9009c 100644 --- a/src/utils/offlineSessionStub.ts +++ b/src/utils/offlineSessionStub.ts @@ -10,7 +10,65 @@ * @module offlineSessionStub */ -import type { ApiSessionClient } from '@/api/apiSession'; +import { EventEmitter } from 'node:events'; +import type { ACPMessageData, ACPProvider, ApiSessionClient } from '@/api/apiSession'; +import { RpcHandlerManager } from '@/api/rpc/RpcHandlerManager'; +import type { AgentState, Metadata, Usage, UserMessage } from '@/api/types'; +import type { RawJSONLines } from '@/claude/types'; + +type ApiSessionClientStubContract = Pick< + ApiSessionClient, + | 'sessionId' + | 'rpcHandlerManager' + | 'sendCodexMessage' + | 'sendAgentMessage' + | 'sendClaudeSessionMessage' + | 'sendSessionEvent' + | 'keepAlive' + | 'sendSessionDeath' + | 'sendUsageData' + | 'updateMetadata' + | 'updateAgentState' + | 'onUserMessage' + | 'flush' + | 'close' +>; + +class OfflineSessionStub extends EventEmitter implements ApiSessionClientStubContract { + readonly sessionId: string; + readonly rpcHandlerManager: RpcHandlerManager; + + constructor(sessionId: string) { + super(); + this.sessionId = sessionId; + this.rpcHandlerManager = new RpcHandlerManager({ + scopePrefix: this.sessionId, + encryptionKey: new Uint8Array(32), + encryptionVariant: 'legacy', + logger: () => undefined, + }); + } + + sendCodexMessage(_body: unknown): void {} + sendAgentMessage(_provider: ACPProvider, _body: ACPMessageData): void {} + sendClaudeSessionMessage(_body: RawJSONLines): void {} + sendSessionEvent( + _event: + | { type: 'switch'; mode: 'local' | 'remote' } + | { type: 'message'; message: string } + | { type: 'permission-mode-changed'; mode: 'default' | 'acceptEdits' | 'bypassPermissions' | 'plan' } + | { type: 'ready' }, + _id?: string + ): void {} + keepAlive(_thinking: boolean, _mode: 'local' | 'remote'): void {} + sendSessionDeath(): void {} + sendUsageData(_usage: Usage): void {} + updateMetadata(_handler: (metadata: Metadata) => Metadata): void {} + updateAgentState(_handler: (metadata: AgentState) => AgentState): void {} + onUserMessage(_callback: (data: UserMessage) => void): void {} + async flush(): Promise {} + async close(): Promise {} +} /** * Creates a no-op session stub for offline mode. @@ -32,23 +90,8 @@ import type { ApiSessionClient } from '@/api/apiSession'; * ``` */ export function createOfflineSessionStub(sessionTag: string): ApiSessionClient { - return { - sessionId: `offline-${sessionTag}`, - sendCodexMessage: () => {}, - sendAgentMessage: () => {}, - sendClaudeSessionMessage: () => {}, - keepAlive: () => {}, - sendSessionEvent: () => {}, - sendSessionDeath: () => {}, - updateLifecycleState: () => {}, - requestControlTransfer: async () => {}, - flush: async () => {}, - close: async () => {}, - updateMetadata: () => {}, - updateAgentState: () => {}, - onUserMessage: () => {}, - rpcHandlerManager: { - registerHandler: () => {} - } - } as unknown as ApiSessionClient; + const stub = new OfflineSessionStub(`offline-${sessionTag}`); + const _typecheck: ApiSessionClientStubContract = stub; + void _typecheck; + return stub as unknown as ApiSessionClient; } diff --git a/src/utils/spawnHappyCLI.invocation.test.ts b/src/utils/spawnHappyCLI.invocation.test.ts new file mode 100644 index 00000000..af79f714 --- /dev/null +++ b/src/utils/spawnHappyCLI.invocation.test.ts @@ -0,0 +1,42 @@ +/** + * Tests for building happy-cli subprocess invocations across runtimes (node/bun). + */ +import { afterEach, describe, it, expect } from 'vitest'; + +describe('happy-cli subprocess invocation', () => { + const originalRuntimeOverride = process.env.HAPPY_CLI_SUBPROCESS_RUNTIME; + + afterEach(() => { + if (originalRuntimeOverride === undefined) { + delete process.env.HAPPY_CLI_SUBPROCESS_RUNTIME; + } else { + process.env.HAPPY_CLI_SUBPROCESS_RUNTIME = originalRuntimeOverride; + } + }); + + it('builds a node invocation by default', async () => { + delete process.env.HAPPY_CLI_SUBPROCESS_RUNTIME; + const mod = (await import('@/utils/spawnHappyCLI')) as typeof import('@/utils/spawnHappyCLI'); + + const inv = mod.buildHappyCliSubprocessInvocation(['--version']); + expect(inv.runtime).toBe('node'); + expect(inv.argv).toEqual( + expect.arrayContaining([ + '--no-warnings', + '--no-deprecation', + expect.stringMatching(/dist\/index\.mjs$/), + '--version', + ]), + ); + }); + + it('builds a bun invocation when HAPPY_CLI_SUBPROCESS_RUNTIME=bun', async () => { + process.env.HAPPY_CLI_SUBPROCESS_RUNTIME = 'bun'; + const mod = (await import('@/utils/spawnHappyCLI')) as typeof import('@/utils/spawnHappyCLI'); + const inv = mod.buildHappyCliSubprocessInvocation(['--version']); + expect(inv.runtime).toBe('bun'); + expect(inv.argv).toEqual(expect.arrayContaining([expect.stringMatching(/dist\/index\.mjs$/), '--version'])); + expect(inv.argv).not.toContain('--no-warnings'); + expect(inv.argv).not.toContain('--no-deprecation'); + }); +}); diff --git a/src/utils/spawnHappyCLI.ts b/src/utils/spawnHappyCLI.ts index 560633ff..ee47d686 100644 --- a/src/utils/spawnHappyCLI.ts +++ b/src/utils/spawnHappyCLI.ts @@ -56,6 +56,36 @@ import { logger } from '@/ui/logger'; import { existsSync } from 'node:fs'; import { isBun } from './runtime'; +function getSubprocessRuntime(): 'node' | 'bun' { + const override = process.env.HAPPY_CLI_SUBPROCESS_RUNTIME; + if (override === 'node' || override === 'bun') return override; + return isBun() ? 'bun' : 'node'; +} + +export function buildHappyCliSubprocessInvocation(args: string[]): { runtime: 'node' | 'bun'; argv: string[] } { + const projectRoot = projectPath(); + const entrypoint = join(projectRoot, 'dist', 'index.mjs'); + + // Use the same Node.js flags that the wrapper script uses + const nodeArgs = [ + '--no-warnings', + '--no-deprecation', + entrypoint, + ...args + ]; + + // Sanity check of the entrypoint path exists + if (!existsSync(entrypoint)) { + const errorMessage = `Entrypoint ${entrypoint} does not exist`; + logger.debug(`[SPAWN HAPPY CLI] ${errorMessage}`); + throw new Error(errorMessage); + } + + const runtime = getSubprocessRuntime(); + const argv = runtime === 'node' ? nodeArgs : [entrypoint, ...args]; + return { runtime, argv }; +} + /** * Spawn the Happy CLI with the given arguments in a cross-platform way. * @@ -68,9 +98,6 @@ import { isBun } from './runtime'; * @returns ChildProcess instance */ export function spawnHappyCLI(args: string[], options: SpawnOptions = {}): ChildProcess { - const projectRoot = projectPath(); - const entrypoint = join(projectRoot, 'dist', 'index.mjs'); - let directory: string | URL | undefined; if ('cwd' in options) { directory = options.cwd @@ -85,21 +112,6 @@ export function spawnHappyCLI(args: string[], options: SpawnOptions = {}): Child const fullCommand = `happy ${args.join(' ')}`; logger.debug(`[SPAWN HAPPY CLI] Spawning: ${fullCommand} in ${directory}`); - // Use the same Node.js flags that the wrapper script uses - const nodeArgs = [ - '--no-warnings', - '--no-deprecation', - entrypoint, - ...args - ]; - - // Sanity check of the entrypoint path exists - if (!existsSync(entrypoint)) { - const errorMessage = `Entrypoint ${entrypoint} does not exist`; - logger.debug(`[SPAWN HAPPY CLI] ${errorMessage}`); - throw new Error(errorMessage); - } - - const runtime = isBun() ? 'bun' : 'node'; - return spawn(runtime, nodeArgs, options); + const { runtime, argv } = buildHappyCliSubprocessInvocation(args); + return spawn(runtime, argv, options); } diff --git a/src/utils/tmux.commandEnv.test.ts b/src/utils/tmux.commandEnv.test.ts new file mode 100644 index 00000000..8649549d --- /dev/null +++ b/src/utils/tmux.commandEnv.test.ts @@ -0,0 +1,60 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { EventEmitter } from 'node:events'; +import type { SpawnOptions, ChildProcessWithoutNullStreams } from 'node:child_process'; + +type SpawnCall = { + command: string; + args: string[]; + options: SpawnOptions; +}; + +const { spawnMock, getLastSpawnCall } = vi.hoisted(() => { + let lastSpawnCall: SpawnCall | null = null; + + const spawnMock = vi.fn((command: string, args: readonly string[], options: SpawnOptions) => { + lastSpawnCall = { command, args: [...args], options }; + + type MinimalChild = EventEmitter & { + stdout: EventEmitter; + stderr: EventEmitter; + }; + + const child = new EventEmitter() as MinimalChild; + child.stdout = new EventEmitter(); + child.stderr = new EventEmitter(); + + queueMicrotask(() => { + child.emit('close', 0); + }); + + return child as unknown as ChildProcessWithoutNullStreams; + }); + + return { + spawnMock, + getLastSpawnCall: () => lastSpawnCall, + }; +}); + +vi.mock('child_process', () => ({ + spawn: spawnMock, +})); + +describe('TmuxUtilities tmux subprocess environment', () => { + beforeEach(() => { + spawnMock.mockClear(); + }); + + it('passes TMUX_TMPDIR to tmux subprocess env when provided', async () => { + vi.resetModules(); + const { TmuxUtilities } = await import('@/utils/tmux'); + + const utils = new TmuxUtilities('happy', { TMUX_TMPDIR: '/custom/tmux' }); + await utils.executeTmuxCommand(['list-sessions']); + + const call = getLastSpawnCall(); + expect(call).not.toBeNull(); + expect((call!.options.env as NodeJS.ProcessEnv | undefined)?.TMUX_TMPDIR).toBe('/custom/tmux'); + }); +}); + diff --git a/src/utils/tmux.real.integration.test.ts b/src/utils/tmux.real.integration.test.ts new file mode 100644 index 00000000..6bcf7db0 --- /dev/null +++ b/src/utils/tmux.real.integration.test.ts @@ -0,0 +1,322 @@ +/** + * Opt-in tmux integration tests. + * + * These tests start isolated tmux servers (via `-S` or `TMUX_TMPDIR`) and must + * never interact with a user's existing tmux sessions. + * + * Enable with: `HAPPY_CLI_TMUX_INTEGRATION=1` + */ + +import { describe, it, expect } from 'vitest'; +import { mkdtempSync, rmSync, writeFileSync, readFileSync, existsSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { spawnSync } from 'node:child_process'; +import { TmuxUtilities } from '@/utils/tmux'; + +function isTmuxInstalled(): boolean { + const result = spawnSync('tmux', ['-V'], { encoding: 'utf8' }); + return result.status === 0; +} + +function shouldRunTmuxIntegration(): boolean { + return process.env.HAPPY_CLI_TMUX_INTEGRATION === '1' && isTmuxInstalled(); +} + +function waitForFile(path: string, timeoutMs: number): Promise { + const pollIntervalMs = 50; + const start = Date.now(); + return new Promise((resolve, reject) => { + const tick = () => { + if (existsSync(path)) return resolve(); + if (Date.now() - start > timeoutMs) { + return reject(new Error(`Timed out waiting for file: ${path}`)); + } + setTimeout(tick, pollIntervalMs); + }; + tick(); + }); +} + +function writeDumpScript(dir: string): string { + const scriptPath = join(dir, 'happy-cli-tmux-dump.cjs'); + writeFileSync( + scriptPath, + [ + "const fs = require('fs');", + "const outFile = process.argv[2];", + "const keepAliveMs = Number(process.argv[3] || '0');", + 'const payload = {', + ' argv: process.argv.slice(4),', + ' env: {', + ' FOO: process.env.FOO,', + ' BAR: process.env.BAR,', + ' TMUX: process.env.TMUX,', + ' TMUX_PANE: process.env.TMUX_PANE,', + ' TMUX_TMPDIR: process.env.TMUX_TMPDIR,', + ' },', + '};', + 'fs.writeFileSync(outFile, JSON.stringify(payload));', + 'if (keepAliveMs > 0) setTimeout(() => {}, keepAliveMs);', + '', + ].join('\n'), + 'utf8', + ); + return scriptPath; +} + +type DumpScriptPayload = { + argv: string[]; + env: { + FOO?: string; + BAR?: string; + TMUX?: string; + TMUX_PANE?: string; + TMUX_TMPDIR?: string; + }; +}; + +function readDumpPayload(outFile: string): DumpScriptPayload { + return JSON.parse(readFileSync(outFile, 'utf8')) as DumpScriptPayload; +} + +async function withCleanTmuxClientEnv(fn: () => Promise): Promise { + const originalTmux = process.env.TMUX; + const originalTmuxPane = process.env.TMUX_PANE; + const originalTmuxTmpDir = process.env.TMUX_TMPDIR; + + delete process.env.TMUX; + delete process.env.TMUX_PANE; + delete process.env.TMUX_TMPDIR; + + try { + return await fn(); + } finally { + if (originalTmux === undefined) delete process.env.TMUX; + else process.env.TMUX = originalTmux; + + if (originalTmuxPane === undefined) delete process.env.TMUX_PANE; + else process.env.TMUX_PANE = originalTmuxPane; + + if (originalTmuxTmpDir === undefined) delete process.env.TMUX_TMPDIR; + else process.env.TMUX_TMPDIR = originalTmuxTmpDir; + } +} + +type TmuxRunResult = { + status: number | null; + stdout: string; + stderr: string; + error: Error | undefined; +}; + +function runTmux(args: string[], options?: { env?: Record }): TmuxRunResult { + // Never inherit the user's existing tmux context (TMUX/TMUX_PANE) or TMUX_TMPDIR. + // These tests must only ever talk to isolated servers created by the test itself. + const env: NodeJS.ProcessEnv = { ...process.env }; + delete env.TMUX; + delete env.TMUX_PANE; + delete env.TMUX_TMPDIR; + + const result = spawnSync('tmux', args, { + encoding: 'utf8', + env: { + ...env, + ...(options?.env ?? {}), + } as NodeJS.ProcessEnv, + }); + return { + status: result.status, + stdout: result.stdout ?? '', + stderr: result.stderr ?? '', + error: result.error, + }; +} + +function killIsolatedTmuxServer(socketPath: string): void { + const result = runTmux(['-S', socketPath, 'kill-server']); + if (result.status !== 0 && process.env.DEBUG) { + // Cleanup should never fail the test run, but debug logging can help diagnose flakes. + console.error('[tmux-it] Failed to kill isolated tmux server', { + socketPath, + status: result.status, + stderr: result.stderr, + error: result.error?.message, + }); + } +} + +describe.skipIf(!shouldRunTmuxIntegration())('tmux (real) integration tests (opt-in)', { timeout: 20_000 }, () => { + it('spawnInTmux returns a real pane PID via -P/-F (regression: PR107 option ordering)', async () => { + const dir = mkdtempSync(join(tmpdir(), 'happy-cli-tmux-it-')); + const socketPath = join(dir, 'tmux.sock'); + const utils = new TmuxUtilities('happy', undefined, socketPath); + + try { + const scriptPath = writeDumpScript(dir); + const outFile = join(dir, 'out.json'); + + const sessionName = `happy-it-${process.pid}-${Date.now()}`; + const windowName = 'pid'; + + const result = await utils.spawnInTmux( + [process.execPath, scriptPath, outFile, '5000', 'pid-check'], + { sessionName, windowName, cwd: dir }, + {}, + ); + + expect(result.success).toBe(true); + expect(typeof result.pid).toBe('number'); + expect(result.pid).toBeGreaterThan(0); + + // Ground truth: query tmux directly for the pane pid. + const panes = runTmux(['-S', socketPath, 'list-panes', '-t', `${sessionName}:${windowName}`, '-F', '#{pane_pid}']); + expect(panes.status).toBe(0); + const listedPid = Number.parseInt(panes.stdout.trim(), 10); + expect(listedPid).toBe(result.pid); + + await waitForFile(outFile, 2_000); + const payload = readDumpPayload(outFile); + expect(payload.argv).toEqual(['pid-check']); + + // Validate the TMUX env format: socket_path,server_pid,pane (not session/window). + expect(typeof payload.env?.TMUX).toBe('string'); + const parts = String(payload.env.TMUX).split(','); + expect(parts.length).toBeGreaterThanOrEqual(3); + expect(parts[0]!.length).toBeGreaterThan(0); + expect(/^\d+$/.test(parts[1]!)).toBe(true); + } finally { + // Kill only the isolated server (never touch the user's default tmux server). + killIsolatedTmuxServer(socketPath); + rmSync(dir, { recursive: true, force: true }); + } + }); + + it('spawnInTmux passes -e KEY=VALUE env values literally (regression: PR107 quoting/escaping)', async () => { + const dir = mkdtempSync(join(tmpdir(), 'happy-cli-tmux-it-')); + const socketPath = join(dir, 'tmux.sock'); + const utils = new TmuxUtilities('happy', undefined, socketPath); + + try { + const scriptPath = writeDumpScript(dir); + const outFile = join(dir, 'out.json'); + + const sessionName = `happy-it-${process.pid}-${Date.now()}`; + const windowName = 'env'; + + const env = { + FOO: 'a$b', + BAR: 'quote"back\\tick`', + }; + + const result = await utils.spawnInTmux( + [process.execPath, scriptPath, outFile, '5000', 'env-check'], + { sessionName, windowName, cwd: dir }, + env, + ); + + expect(result.success).toBe(true); + + await waitForFile(outFile, 2_000); + const payload = readDumpPayload(outFile); + + expect(payload.env?.FOO).toBe(env.FOO); + expect(payload.env?.BAR).toBe(env.BAR); + } finally { + killIsolatedTmuxServer(socketPath); + rmSync(dir, { recursive: true, force: true }); + } + }); + + it('spawnInTmux quotes command tokens safely (regression: PR107 args.join(\" \") injection/splitting)', async () => { + const dir = mkdtempSync(join(tmpdir(), 'happy-cli-tmux-it-')); + const socketPath = join(dir, 'tmux.sock'); + const utils = new TmuxUtilities('happy', undefined, socketPath); + + try { + const scriptPath = writeDumpScript(dir); + const outFile = join(dir, 'out.json'); + const sentinelFile = join(dir, 'injection-sentinel'); + + const sessionName = `happy-it-${process.pid}-${Date.now()}`; + const windowName = 'quote'; + + const argWithSpaces = 'a b'; + const argWithSingleQuote = "c'd"; + const injectionArg = `$(touch ${sentinelFile})`; + + const result = await utils.spawnInTmux( + [process.execPath, scriptPath, outFile, '5000', argWithSpaces, argWithSingleQuote, injectionArg], + { sessionName, windowName, cwd: dir }, + {}, + ); + + expect(result.success).toBe(true); + + await waitForFile(outFile, 2_000); + const payload = readDumpPayload(outFile); + expect(payload.argv).toEqual([argWithSpaces, argWithSingleQuote, injectionArg]); + + // If quoting were broken, the shell would execute `touch ` and create the file. + expect(existsSync(sentinelFile)).toBe(false); + } finally { + killIsolatedTmuxServer(socketPath); + rmSync(dir, { recursive: true, force: true }); + } + }); + + it('TMUX_TMPDIR affects which tmux server commands talk to (regression: PR107 wrong-server assumptions)', async () => { + const dir = mkdtempSync(join(tmpdir(), 'happy-cli-tmux-it-')); + // IMPORTANT: keep the socket path short to avoid unix domain socket length limits (common on macOS). + // tmux will create tmux-/default within this directory. + const tmuxTmpDir = mkdtempSync(join(tmpdir(), 'happy-cli-tmux-tmpdir-it-')); + + const utils = new TmuxUtilities('happy', { TMUX_TMPDIR: tmuxTmpDir }); + + try { + const scriptPath = writeDumpScript(dir); + const outFile = join(dir, 'out.json'); + + const sessionName = `happy-it-${process.pid}-${Date.now()}`; + const windowName = 'tmpdir'; + + const result = await withCleanTmuxClientEnv(() => + utils.spawnInTmux( + [process.execPath, scriptPath, outFile, '5000', 'tmpdir-check'], + { sessionName, windowName, cwd: dir }, + {}, + ), + ); + + if (!result.success) { + throw new Error(`spawnInTmux failed: ${result.error ?? 'unknown error'}`); + } + + // Without TMUX_TMPDIR, a fresh tmux client should not see the isolated session. + const defaultList = runTmux(['list-sessions']); + expect(defaultList.stdout.includes(sessionName)).toBe(false); + + // With TMUX_TMPDIR, tmux should see our isolated session. + const isolatedList = runTmux(['list-sessions'], { env: { TMUX_TMPDIR: tmuxTmpDir } }); + expect(isolatedList.status).toBe(0); + expect(isolatedList.stdout.includes(sessionName)).toBe(true); + + await waitForFile(outFile, 2_000); + const payload = readDumpPayload(outFile); + expect(payload.argv).toEqual(['tmpdir-check']); + } finally { + // Kill only the isolated server identified by TMUX_TMPDIR. + const result = runTmux(['kill-server'], { env: { TMUX_TMPDIR: tmuxTmpDir } }); + if (result.status !== 0 && process.env.DEBUG) { + console.error('[tmux-it] Failed to kill isolated tmux server via TMUX_TMPDIR', { + tmuxTmpDir, + status: result.status, + stderr: result.stderr, + error: result.error?.message, + }); + } + rmSync(tmuxTmpDir, { recursive: true, force: true }); + rmSync(dir, { recursive: true, force: true }); + } + }); +}); diff --git a/src/utils/tmux.socketPath.test.ts b/src/utils/tmux.socketPath.test.ts new file mode 100644 index 00000000..3afb096b --- /dev/null +++ b/src/utils/tmux.socketPath.test.ts @@ -0,0 +1,62 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { EventEmitter } from 'node:events'; +import type { SpawnOptions, ChildProcessWithoutNullStreams } from 'node:child_process'; + +type SpawnCall = { + command: string; + args: string[]; + options: SpawnOptions; +}; + +const { spawnMock, getLastSpawnCall } = vi.hoisted(() => { + let lastSpawnCall: SpawnCall | null = null; + + const spawnMock = vi.fn((command: string, args: readonly string[], options: SpawnOptions) => { + lastSpawnCall = { command, args: [...args], options }; + + type MinimalChild = EventEmitter & { + stdout: EventEmitter; + stderr: EventEmitter; + }; + + const child = new EventEmitter() as MinimalChild; + child.stdout = new EventEmitter(); + child.stderr = new EventEmitter(); + + queueMicrotask(() => { + child.emit('close', 0); + }); + + return child as unknown as ChildProcessWithoutNullStreams; + }); + + return { + spawnMock, + getLastSpawnCall: () => lastSpawnCall, + }; +}); + +vi.mock('child_process', () => ({ + spawn: spawnMock, +})); + +describe('TmuxUtilities tmux socket path', () => { + beforeEach(() => { + spawnMock.mockClear(); + }); + + it('uses -S by default when configured', async () => { + vi.resetModules(); + const { TmuxUtilities } = await import('@/utils/tmux'); + + const socketPath = '/tmp/happy-cli-tmux-test.sock'; + const utils = new TmuxUtilities('happy', undefined, socketPath); + await utils.executeTmuxCommand(['list-sessions']); + + const call = getLastSpawnCall(); + expect(call).not.toBeNull(); + expect(call!.command).toBe('tmux'); + expect(call!.args).toEqual(expect.arrayContaining(['-S', socketPath])); + }); +}); + diff --git a/src/utils/tmux.test.ts b/src/utils/tmux.test.ts index c5628e98..6b4f343a 100644 --- a/src/utils/tmux.test.ts +++ b/src/utils/tmux.test.ts @@ -5,17 +5,32 @@ * They do NOT require tmux to be installed on the system. * All tests mock environment variables and test string parsing only. */ -import { describe, expect, it } from 'vitest'; +import { describe, expect, it, vi } from 'vitest'; import { + normalizeExitCode, parseTmuxSessionIdentifier, formatTmuxSessionIdentifier, validateTmuxSessionIdentifier, buildTmuxSessionIdentifier, + createTmuxSession, TmuxSessionIdentifierError, + extractSessionAndWindow, TmuxUtilities, type TmuxSessionIdentifier, + type TmuxCommandResult, } from './tmux'; +describe('normalizeExitCode', () => { + it('treats signal termination (null) as non-zero', () => { + expect(normalizeExitCode(null)).toBe(1); + }); + + it('preserves normal exit codes', () => { + expect(normalizeExitCode(0)).toBe(0); + expect(normalizeExitCode(2)).toBe(2); + }); +}); + describe('parseTmuxSessionIdentifier', () => { it('should parse session-only identifier', () => { const result = parseTmuxSessionIdentifier('my-session'); @@ -66,9 +81,12 @@ describe('parseTmuxSessionIdentifier', () => { expect(() => parseTmuxSessionIdentifier(undefined as any)).toThrow(TmuxSessionIdentifierError); }); - it('should throw on invalid session name characters', () => { - expect(() => parseTmuxSessionIdentifier('invalid session')).toThrow(TmuxSessionIdentifierError); - expect(() => parseTmuxSessionIdentifier('invalid session')).toThrow('Only alphanumeric characters, dots, hyphens, and underscores are allowed'); + it('should allow session names with spaces', () => { + const result = parseTmuxSessionIdentifier('my session:window-1'); + expect(result).toEqual({ + session: 'my session', + window: 'window-1', + }); }); it('should throw on special characters in session name', () => { @@ -78,8 +96,8 @@ describe('parseTmuxSessionIdentifier', () => { }); it('should throw on invalid window name characters', () => { - expect(() => parseTmuxSessionIdentifier('session:invalid window')).toThrow(TmuxSessionIdentifierError); - expect(() => parseTmuxSessionIdentifier('session:invalid window')).toThrow('Only alphanumeric characters, dots, hyphens, and underscores are allowed'); + expect(() => parseTmuxSessionIdentifier('session:invalid@window')).toThrow(TmuxSessionIdentifierError); + expect(() => parseTmuxSessionIdentifier('session:invalid@window')).toThrow('Only alphanumeric characters'); }); it('should throw on non-numeric pane identifier', () => { @@ -172,13 +190,13 @@ describe('validateTmuxSessionIdentifier', () => { }); it('should return valid:false for invalid session characters', () => { - const result = validateTmuxSessionIdentifier('invalid session'); + const result = validateTmuxSessionIdentifier('invalid@session'); expect(result.valid).toBe(false); expect(result.error).toContain('Only alphanumeric characters'); }); it('should return valid:false for invalid window characters', () => { - const result = validateTmuxSessionIdentifier('session:invalid window'); + const result = validateTmuxSessionIdentifier('session:invalid@window'); expect(result.valid).toBe(false); expect(result.error).toContain('Only alphanumeric characters'); }); @@ -196,7 +214,7 @@ describe('validateTmuxSessionIdentifier', () => { it('should not throw exceptions', () => { expect(() => validateTmuxSessionIdentifier('')).not.toThrow(); - expect(() => validateTmuxSessionIdentifier('invalid session')).not.toThrow(); + expect(() => validateTmuxSessionIdentifier('invalid@session')).not.toThrow(); expect(() => validateTmuxSessionIdentifier(null as any)).not.toThrow(); }); }); @@ -240,7 +258,7 @@ describe('buildTmuxSessionIdentifier', () => { }); it('should return error for invalid session characters', () => { - const result = buildTmuxSessionIdentifier({ session: 'invalid session' }); + const result = buildTmuxSessionIdentifier({ session: 'invalid@session' }); expect(result.success).toBe(false); expect(result.error).toContain('Invalid session name'); }); @@ -248,7 +266,7 @@ describe('buildTmuxSessionIdentifier', () => { it('should return error for invalid window characters', () => { const result = buildTmuxSessionIdentifier({ session: 'session', - window: 'invalid window' + window: 'invalid@window' }); expect(result.success).toBe(false); expect(result.error).toContain('Invalid window name'); @@ -278,17 +296,23 @@ describe('buildTmuxSessionIdentifier', () => { it('should not throw exceptions for invalid inputs', () => { expect(() => buildTmuxSessionIdentifier({ session: '' })).not.toThrow(); - expect(() => buildTmuxSessionIdentifier({ session: 'invalid session' })).not.toThrow(); + expect(() => buildTmuxSessionIdentifier({ session: 'invalid@session' })).not.toThrow(); expect(() => buildTmuxSessionIdentifier({ session: null as any })).not.toThrow(); }); }); describe('TmuxUtilities.detectTmuxEnvironment', () => { const originalTmuxEnv = process.env.TMUX; + const originalTmuxPaneEnv = process.env.TMUX_PANE; // Helper to set and restore environment - const withTmuxEnv = (value: string | undefined, fn: () => void) => { + const withTmuxEnv = (value: string | undefined, fn: () => void, pane?: string | undefined) => { process.env.TMUX = value; + if (pane !== undefined) { + process.env.TMUX_PANE = pane; + } else { + delete process.env.TMUX_PANE; + } try { fn(); } finally { @@ -297,6 +321,11 @@ describe('TmuxUtilities.detectTmuxEnvironment', () => { } else { delete process.env.TMUX; } + if (originalTmuxPaneEnv !== undefined) { + process.env.TMUX_PANE = originalTmuxPaneEnv; + } else { + delete process.env.TMUX_PANE; + } } }; @@ -313,37 +342,26 @@ describe('TmuxUtilities.detectTmuxEnvironment', () => { const utils = new TmuxUtilities(); const result = utils.detectTmuxEnvironment(); expect(result).toEqual({ - session: '4219', - window: '0', + socket_path: '/tmp/tmux-1000/default', + server_pid: 4219, pane: '0', - socket_path: '/tmp/tmux-1000/default' }); }); }); - it('should parse TMUX env with session.window format', () => { + it('should return null for malformed TMUX env (non-numeric server pid)', () => { withTmuxEnv('/tmp/tmux-1000/default,mysession.mywindow,2', () => { const utils = new TmuxUtilities(); const result = utils.detectTmuxEnvironment(); - expect(result).toEqual({ - session: 'mysession', - window: 'mywindow', - pane: '2', - socket_path: '/tmp/tmux-1000/default' - }); + expect(result).toBeNull(); }); }); - it('should handle TMUX env without session.window format', () => { + it('should return null for malformed TMUX env (non-numeric server pid, no dot)', () => { withTmuxEnv('/tmp/tmux-1000/default,session123,1', () => { const utils = new TmuxUtilities(); const result = utils.detectTmuxEnvironment(); - expect(result).toEqual({ - session: 'session123', - window: '0', - pane: '1', - socket_path: '/tmp/tmux-1000/default' - }); + expect(result).toBeNull(); }); }); @@ -353,24 +371,21 @@ describe('TmuxUtilities.detectTmuxEnvironment', () => { const utils = new TmuxUtilities(); const result = utils.detectTmuxEnvironment(); expect(result).toEqual({ - session: '5678', - window: '0', + socket_path: '/tmp/tmux-1000/my-socket', + server_pid: 5678, pane: '3', - socket_path: '/tmp/tmux-1000/my-socket' }); }); }); it('should handle socket path with multiple slashes', () => { - // Test the array indexing fix - ensure we get the last component correctly - withTmuxEnv('/var/run/tmux/1000/default,session.window,0', () => { + withTmuxEnv('/var/run/tmux/1000/default,1234,0', () => { const utils = new TmuxUtilities(); const result = utils.detectTmuxEnvironment(); expect(result).toEqual({ - session: 'session', - window: 'window', + socket_path: '/var/run/tmux/1000/default', + server_pid: 1234, pane: '0', - socket_path: '/var/run/tmux/1000/default' }); }); }); @@ -397,10 +412,9 @@ describe('TmuxUtilities.detectTmuxEnvironment', () => { const result = utils.detectTmuxEnvironment(); // Should still parse the first 3 parts correctly expect(result).toEqual({ - session: '4219', - window: '0', + socket_path: '/tmp/tmux-1000/default', + server_pid: 4219, pane: '0', - socket_path: '/tmp/tmux-1000/default' }); }); }); @@ -409,14 +423,20 @@ describe('TmuxUtilities.detectTmuxEnvironment', () => { withTmuxEnv('/tmp/tmux-1000/default,my.session.name.5,2', () => { const utils = new TmuxUtilities(); const result = utils.detectTmuxEnvironment(); - // Split on dot, so my.session becomes session=my, window=session + expect(result).toBeNull(); + }); + }); + + it('should prefer TMUX_PANE (pane id) when present', () => { + withTmuxEnv('/tmp/tmux-1000/default,4219,0', () => { + const utils = new TmuxUtilities(); + const result = utils.detectTmuxEnvironment(); expect(result).toEqual({ - session: 'my', - window: 'session', - pane: '2', - socket_path: '/tmp/tmux-1000/default' + socket_path: '/tmp/tmux-1000/default', + server_pid: 4219, + pane: '%0', }); - }); + }, '%0'); }); }); @@ -454,3 +474,148 @@ describe('Round-trip consistency', () => { expect(parsed).toEqual(params); }); }); + +describe('extractSessionAndWindow', () => { + it('extracts session and window names containing spaces', () => { + const parsed = extractSessionAndWindow('my session:my window.2'); + expect(parsed).toEqual({ session: 'my session', window: 'my window' }); + }); +}); + +describe('createTmuxSession', () => { + it('returns a trimmed session identifier', async () => { + const spy = vi + .spyOn(TmuxUtilities.prototype, 'executeTmuxCommand') + .mockResolvedValue({ returncode: 0, stdout: '', stderr: '', command: [] }); + + try { + const result = await createTmuxSession(' my session ', { windowName: 'main' }); + expect(result.success).toBe(true); + expect(result.sessionIdentifier).toBe('my session:main'); + } finally { + spy.mockRestore(); + } + }); +}); + +describe('TmuxUtilities.spawnInTmux', () => { + class FakeTmuxUtilities extends TmuxUtilities { + public calls: Array<{ cmd: string[]; session?: string }> = []; + + async executeTmuxCommand(cmd: string[], session?: string): Promise { + this.calls.push({ cmd, session }); + + if (cmd[0] === 'list-sessions') { + // tmux availability check + if (cmd.length === 1) { + return { returncode: 0, stdout: 'oldSess: 1 windows\nnewSess: 2 windows\n', stderr: '', command: cmd }; + } + + // Most-recent selection format + if (cmd[1] === '-F' && cmd[2]?.includes('session_last_attached')) { + return { + returncode: 0, + stdout: 'oldSess\t0\t100\nnewSess\t0\t200\n', + stderr: '', + command: cmd, + }; + } + + // Legacy name-only listing + if (cmd[1] === '-F') { + return { returncode: 0, stdout: 'oldSess\nnewSess\n', stderr: '', command: cmd }; + } + } + + if (cmd[0] === 'has-session') { + return { returncode: 0, stdout: '', stderr: '', command: cmd }; + } + + if (cmd[0] === 'new-session') { + return { returncode: 0, stdout: '', stderr: '', command: cmd }; + } + + if (cmd[0] === 'new-window') { + return { returncode: 0, stdout: '4242\n', stderr: '', command: cmd }; + } + + return { returncode: 0, stdout: '', stderr: '', command: cmd }; + } + } + + it('builds tmux new-window args without quoting env values', async () => { + const tmux = new FakeTmuxUtilities(); + + await tmux.spawnInTmux( + ['echo', 'hello'], + { sessionName: 'my-session', windowName: 'my-window', cwd: '/tmp' }, + { FOO: 'a$b', BAR: 'quote"back\\tick`' } + ); + + const newWindowCall = tmux.calls.find((call) => call.cmd[0] === 'new-window'); + expect(newWindowCall).toBeDefined(); + + const newWindowArgs = newWindowCall!.cmd; + + // -e takes literal KEY=VALUE, not shell-escaped values. + expect(newWindowArgs).toContain('FOO=a$b'); + expect(newWindowArgs).toContain('BAR=quote"back\\tick`'); + expect(newWindowArgs.some((arg) => arg.startsWith('FOO="'))).toBe(false); + expect(newWindowArgs.some((arg) => arg.startsWith('BAR="'))).toBe(false); + + // -P/-F options must appear before the shell command argument. + const commandIndex = newWindowArgs.indexOf("'echo' 'hello'"); + const pIndex = newWindowArgs.indexOf('-P'); + const fIndex = newWindowArgs.indexOf('-F'); + expect(pIndex).toBeGreaterThanOrEqual(0); + expect(fIndex).toBeGreaterThanOrEqual(0); + expect(commandIndex).toBeGreaterThanOrEqual(0); + expect(pIndex).toBeLessThan(commandIndex); + expect(fIndex).toBeLessThan(commandIndex); + + // When targeting a specific session, -t must be included explicitly. + const tIndex = newWindowArgs.indexOf('-t'); + expect(tIndex).toBeGreaterThanOrEqual(0); + expect(newWindowArgs[tIndex + 1]).toBe('my-session'); + expect(tIndex).toBeLessThan(commandIndex); + }); + + it('quotes command arguments for tmux shell command safely', async () => { + const tmux = new FakeTmuxUtilities(); + + await tmux.spawnInTmux( + ['echo', 'a b', "c'd", '$(rm -rf /)'], + { sessionName: 'my-session', windowName: 'my-window' }, + {} + ); + + const newWindowCall = tmux.calls.find((call) => call.cmd[0] === 'new-window'); + expect(newWindowCall).toBeDefined(); + + const newWindowArgs = newWindowCall!.cmd; + const commandArg = newWindowArgs[newWindowArgs.length - 1]; + expect(commandArg).toBe("'echo' 'a b' 'c'\\''d' '$(rm -rf /)'"); + }); + + it('treats empty sessionName as current/most-recent session (deterministic)', async () => { + const tmux = new FakeTmuxUtilities(); + + const result = await tmux.spawnInTmux( + ['echo', 'hello'], + { sessionName: '', windowName: 'my-window' }, + {} + ); + + expect(result.success).toBe(true); + expect(result.sessionId).toBe('newSess:my-window'); + + // Should request deterministic session selection metadata (not just "first session") + const usedLastAttachedFormat = tmux.calls.some( + (call) => + call.cmd[0] === 'list-sessions' && + call.cmd[1] === '-F' && + Boolean(call.cmd[2]?.includes('session_last_attached')) + ); + expect(usedLastAttachedFormat).toBe(true); + }); +}); diff --git a/src/utils/tmux.ts b/src/utils/tmux.ts index f0958358..52ede652 100644 --- a/src/utils/tmux.ts +++ b/src/utils/tmux.ts @@ -23,6 +23,21 @@ import { spawn, SpawnOptions } from 'child_process'; import { promisify } from 'util'; import { logger } from '@/ui/logger'; +export function normalizeExitCode(code: number | null): number { + // Node passes `code === null` when the process was terminated by a signal. + // Preserve failure semantics rather than treating it as success. + return code ?? 1; +} + +function quoteForPosixShell(arg: string): string { + // POSIX-safe single-quote escaping: ' -> '\'' . + return `'${arg.replace(/'/g, `'\\''`)}'`; +} + +function buildPosixShellCommand(args: string[]): string { + return args.map(quoteForPosixShell).join(' '); +} + export enum TmuxControlState { /** Normal text processing mode */ NORMAL = "normal", @@ -77,10 +92,12 @@ export type TmuxWindowOperation = | 'join-pane' | 'join' | 'break-pane' | 'break'; export interface TmuxEnvironment { - session: string; - window: string; + /** tmux server socket path (TMUX env var first component) */ + socket_path: string; + /** tmux server pid (TMUX env var second component) */ + server_pid: number; + /** tmux pane identifier/index (TMUX env var third component) */ pane: string; - socket_path?: string; } export interface TmuxCommandResult { @@ -98,8 +115,6 @@ export interface TmuxSessionInfo { socket_path?: string; tmux_active: boolean; current_session?: string; - env_session?: string; - env_window?: string; env_pane?: string; available_sessions: string[]; } @@ -135,17 +150,19 @@ export function parseTmuxSessionIdentifier(identifier: string): TmuxSessionIdent session: parts[0].trim() }; - // Validate session name (tmux has restrictions on session names) - if (!/^[a-zA-Z0-9._-]+$/.test(result.session)) { - throw new TmuxSessionIdentifierError(`Invalid session name: "${result.session}". Only alphanumeric characters, dots, hyphens, and underscores are allowed.`); + // Validate session name for our identifier format. + // Allow spaces, since tmux sessions can be user-named with spaces. + // Disallow characters that would make our identifier ambiguous (e.g. ':' separator). + if (!/^[a-zA-Z0-9._ -]+$/.test(result.session)) { + throw new TmuxSessionIdentifierError(`Invalid session name: "${result.session}". Only alphanumeric characters, spaces, dots, hyphens, and underscores are allowed.`); } if (parts.length > 1) { const windowAndPane = parts[1].split('.'); result.window = windowAndPane[0]?.trim(); - if (result.window && !/^[a-zA-Z0-9._-]+$/.test(result.window)) { - throw new TmuxSessionIdentifierError(`Invalid window name: "${result.window}". Only alphanumeric characters, dots, hyphens, and underscores are allowed.`); + if (result.window && !/^[a-zA-Z0-9._ -]+$/.test(result.window)) { + throw new TmuxSessionIdentifierError(`Invalid window name: "${result.window}". Only alphanumeric characters, spaces, dots, hyphens, and underscores are allowed.`); } if (windowAndPane.length > 1) { @@ -183,15 +200,25 @@ export function extractSessionAndWindow(tmuxOutput: string): { session: string; // Look for session:window patterns in tmux output const lines = tmuxOutput.split('\n'); + const nameRegex = /^[a-zA-Z0-9._ -]+$/; for (const line of lines) { - const match = line.match(/^([a-zA-Z0-9._-]+):([a-zA-Z0-9._-]+)(?:\.([0-9]+))?/); - if (match) { - return { - session: match[1], - window: match[2] - }; - } + const trimmed = line.trim(); + if (!trimmed) continue; + + // Allow spaces in names, but keep ':' as the session/window separator. + // This helper is intended for extracting the canonical identifier shapes that tmux can emit + // via format strings (e.g. '#S:#W' or '#S:#W.#P'), so we require end-of-line matches. + const match = trimmed.match(/^(.+?):(.+?)(?:\.([0-9]+))?$/); + if (!match) continue; + + const session = match[1]?.trim(); + const window = match[2]?.trim(); + + if (!session || !window) continue; + if (!nameRegex.test(session) || !nameRegex.test(window)) continue; + + return { session, window }; } return null; @@ -343,7 +370,8 @@ const COMMANDS_SUPPORTING_TARGET = new Set([ 'send-keys', 'capture-pane', 'new-window', 'kill-window', 'select-window', 'split-window', 'select-pane', 'kill-pane', 'select-layout', 'display-message', 'attach-session', 'detach-client', - 'new-session', 'kill-session', 'list-windows', 'list-panes' + // NOTE: `new-session -t` targets a *group name*, not a session/window target. + 'kill-session', 'list-windows', 'list-panes' ]); // Control sequences that must be separate arguments with proper typing @@ -359,9 +387,13 @@ export class TmuxUtilities { private controlState: TmuxControlState = TmuxControlState.NORMAL; public readonly sessionName: string; + private readonly tmuxCommandEnv?: Record; + private readonly tmuxSocketPath?: string; - constructor(sessionName?: string) { + constructor(sessionName?: string, tmuxCommandEnv?: Record, tmuxSocketPath?: string) { this.sessionName = sessionName || TmuxUtilities.DEFAULT_SESSION_NAME; + this.tmuxCommandEnv = tmuxCommandEnv; + this.tmuxSocketPath = tmuxSocketPath; } /** @@ -373,35 +405,25 @@ export class TmuxUtilities { return null; } - // Parse TMUX environment: /tmp/tmux-1000/default,4219,0 + // TMUX environment format: socket_path,server_pid,pane_id + // NOTE: session name / window are NOT encoded in TMUX. Query tmux formats for those. try { const parts = tmuxEnv.split(','); - if (parts.length >= 3) { - const socketPath = parts[0]; - // Extract last component from path (JavaScript doesn't support negative array indexing) - const pathParts = parts[1].split('/'); - const sessionAndWindow = pathParts[pathParts.length - 1] || parts[1]; - const pane = parts[2]; - - // Extract session name from session.window format - let session: string; - let window: string; - if (sessionAndWindow.includes('.')) { - const parts = sessionAndWindow.split('.', 2); - session = parts[0]; - window = parts[1] || "0"; - } else { - session = sessionAndWindow; - window = "0"; - } + if (parts.length < 3) return null; - return { - session, - window, - pane, - socket_path: socketPath - }; - } + const socketPath = parts[0]?.trim(); + const serverPidStr = parts[1]?.trim(); + // Prefer TMUX_PANE (pane id like %0). Fallback to TMUX env var third component (often pane index). + const pane = (process.env.TMUX_PANE ?? parts[2])?.trim(); + + if (!socketPath || !serverPidStr || !pane) return null; + if (!/^\d+$/.test(serverPidStr)) return null; + + return { + socket_path: socketPath, + server_pid: Number.parseInt(serverPidStr, 10), + pane, + }; } catch (error) { logger.debug('[TMUX] Failed to parse TMUX environment variable:', error); } @@ -425,8 +447,9 @@ export class TmuxUtilities { let baseCmd = ['tmux']; // Add socket specification if provided - if (socketPath) { - baseCmd = ['tmux', '-S', socketPath]; + const resolvedSocketPath = socketPath ?? this.tmuxSocketPath; + if (resolvedSocketPath) { + baseCmd = ['tmux', '-S', resolvedSocketPath]; } // Handle send-keys with proper target specification @@ -448,7 +471,8 @@ export class TmuxUtilities { const fullCmd = [...baseCmd, ...cmd]; // Add target specification for commands that support it - if (cmd.length > 0 && COMMANDS_SUPPORTING_TARGET.has(cmd[0])) { + const hasExplicitTarget = cmd.includes('-t'); + if (!hasExplicitTarget && cmd.length > 0 && COMMANDS_SUPPORTING_TARGET.has(cmd[0])) { let target = targetSession; if (window) target += `:${window}`; if (pane) target += `.${pane}`; @@ -482,11 +506,18 @@ export class TmuxUtilities { */ private runCommand(args: string[], options: SpawnOptions = {}): Promise<{ exitCode: number; stdout: string; stderr: string }> { return new Promise((resolve, reject) => { + const mergedEnv = { + ...process.env, + ...(this.tmuxCommandEnv ?? {}), + ...(options.env ?? {}), + }; + const child = spawn(args[0], args.slice(1), { stdio: ['ignore', 'pipe', 'pipe'], timeout: 5000, shell: false, - ...options + ...options, + env: mergedEnv, }); let stdout = ''; @@ -502,7 +533,7 @@ export class TmuxUtilities { child.on('close', (code) => { resolve({ - exitCode: code || 0, + exitCode: normalizeExitCode(code), stdout, stderr }); @@ -698,19 +729,12 @@ export class TmuxUtilities { pane: "unknown", socket_path: undefined, tmux_active: envInfo !== null, - current_session: envInfo?.session, + current_session: undefined, available_sessions: [] }; - // Update with environment info if it matches our target session - if (envInfo && envInfo.session === targetSession) { - info.window = envInfo.window; - info.pane = envInfo.pane; + if (envInfo) { info.socket_path = envInfo.socket_path; - } else if (envInfo) { - // Add environment info as separate fields - info.env_session = envInfo.session; - info.env_window = envInfo.window; info.env_pane = envInfo.pane; } @@ -731,21 +755,20 @@ export class TmuxUtilities { * Spawn process in tmux session with environment variables. * * IMPORTANT: Unlike Node.js spawn(), env is a separate parameter. - * This is intentional because: - * - Tmux windows inherit environment from the tmux server - * - Only NEW or DIFFERENT variables need to be set via -e flag - * - Passing all of process.env would create 50+ unnecessary -e flags + * This is intentional because tmux sets window-scoped environment via `new-window -e KEY=VALUE`. + * Callers may provide a fully merged environment (daemon env + profile overrides) so tmux and + * non-tmux spawns behave consistently. * * @param args - Command and arguments to execute (as array, will be joined) * @param options - Spawn options (tmux-specific, excludes env) - * @param env - Environment variables to set in window (only pass what's different!) + * @param env - Environment variables to set in window * @returns Result with success status and session identifier */ async spawnInTmux( args: string[], options: TmuxSpawnOptions = {}, env?: Record - ): Promise<{ success: boolean; sessionId?: string; pid?: number; error?: string }> { + ): Promise<{ success: boolean; sessionId?: string; sessionName?: string; windowName?: string; pid?: number; error?: string }> { try { // Check if tmux is available const tmuxCheck = await this.executeTmuxCommand(['list-sessions']); @@ -754,26 +777,41 @@ export class TmuxUtilities { } // Handle session name resolution - // - undefined: Use first existing session or create "happy" - // - empty string: Use first existing session or create "happy" + // - undefined: Use this instance's default session ("happy") + // - empty string: Use current/most-recent session deterministically // - specific name: Use that session (create if doesn't exist) - let sessionName = options.sessionName !== undefined && options.sessionName !== '' - ? options.sessionName - : null; - - // If no specific session name, try to use first existing session - if (!sessionName) { - const listResult = await this.executeTmuxCommand(['list-sessions', '-F', '#{session_name}']); - if (listResult && listResult.returncode === 0 && listResult.stdout.trim()) { - // Use first session from list - const firstSession = listResult.stdout.trim().split('\n')[0]; - sessionName = firstSession; - logger.debug(`[TMUX] Using first existing session: ${sessionName}`); - } else { - // No sessions exist, create "happy" - sessionName = 'happy'; - logger.debug(`[TMUX] No existing sessions, using default: ${sessionName}`); - } + let sessionName = options.sessionName ?? this.sessionName; + + if (options.sessionName === '') { + const listResult = await this.executeTmuxCommand([ + 'list-sessions', + '-F', + '#{session_name}\t#{session_attached}\t#{session_last_attached}', + ]); + + const candidates = (listResult?.stdout ?? '') + .trim() + .split('\n') + .filter((line) => line.trim()) + .map((line) => { + const [name, attachedRaw, lastAttachedRaw] = line.split('\t'); + const attached = Number.parseInt(attachedRaw ?? '0', 10); + const lastAttached = Number.parseInt(lastAttachedRaw ?? '0', 10); + return { + name: (name ?? '').trim(), + attached: Number.isFinite(attached) ? attached : 0, + lastAttached: Number.isFinite(lastAttached) ? lastAttached : 0, + }; + }) + .filter((row) => row.name.length > 0); + + candidates.sort((a, b) => { + // Prefer attached sessions first, then most recently attached. + if (a.attached !== b.attached) return b.attached - a.attached; + return b.lastAttached - a.lastAttached; + }); + + sessionName = candidates[0]?.name ?? TmuxUtilities.DEFAULT_SESSION_NAME; } const windowName = options.windowName || `happy-${Date.now()}`; @@ -782,11 +820,11 @@ export class TmuxUtilities { await this.ensureSessionExists(sessionName); // Build command to execute in the new window - const fullCommand = args.join(' '); + const fullCommand = buildPosixShellCommand(args); // Create new window in session with command and environment variables // IMPORTANT: Don't manually add -t here - executeTmuxCommand handles it via parameters - const createWindowArgs = ['new-window', '-n', windowName]; + const createWindowArgs = ['new-window', '-P', '-F', '#{pane_pid}', '-n', windowName]; // Add working directory if specified if (options.cwd) { @@ -794,6 +832,9 @@ export class TmuxUtilities { createWindowArgs.push('-c', cwdPath); } + // Add target session explicitly so option ordering is correct. + createWindowArgs.push('-t', sessionName); + // Add environment variables using -e flag (sets them in the window's environment) // Note: tmux windows inherit environment from tmux server, but we need to ensure // the daemon's environment variables (especially expanded auth variables) are available @@ -811,15 +852,9 @@ export class TmuxUtilities { continue; } - // Escape value for shell safety - // Must escape: backslashes, double quotes, dollar signs, backticks - const escapedValue = value - .replace(/\\/g, '\\\\') // Backslash first! - .replace(/"/g, '\\"') // Double quotes - .replace(/\$/g, '\\$') // Dollar signs - .replace(/`/g, '\\`'); // Backticks - - createWindowArgs.push('-e', `${key}="${escapedValue}"`); + // `new-window -e` takes KEY=VALUE literally (no shell parsing). + // Do NOT quote or escape values intended for shell parsing. + createWindowArgs.push('-e', `${key}=${value}`); } logger.debug(`[TMUX] Setting ${Object.keys(env).length} environment variables in tmux window`); } @@ -827,12 +862,8 @@ export class TmuxUtilities { // Add the command to run in the window (runs immediately when window is created) createWindowArgs.push(fullCommand); - // Add -P flag to print the pane PID immediately - createWindowArgs.push('-P'); - createWindowArgs.push('-F', '#{pane_pid}'); - // Create window with command and get PID immediately - const createResult = await this.executeTmuxCommand(createWindowArgs, sessionName); + const createResult = await this.executeTmuxCommand(createWindowArgs); if (!createResult || createResult.returncode !== 0) { throw new Error(`Failed to create tmux window: ${createResult?.stderr}`); @@ -855,6 +886,8 @@ export class TmuxUtilities { return { success: true, sessionId: formatTmuxSessionIdentifier(sessionIdentifier), + sessionName, + windowName, pid: panePid }; } catch (error) { @@ -894,8 +927,8 @@ export class TmuxUtilities { throw new TmuxSessionIdentifierError(`Window identifier required: ${sessionIdentifier}`); } - const result = await this.executeWinOp('kill-window', [parsed.window], parsed.session); - return result; + const result = await this.executeTmuxCommand(['kill-window'], parsed.session, parsed.window); + return result !== null && result.returncode === 0; } catch (error) { if (error instanceof TmuxSessionIdentifierError) { logger.debug(`[TMUX] Invalid window identifier: ${error.message}`); @@ -911,24 +944,16 @@ export class TmuxUtilities { */ async listWindows(sessionName?: string): Promise { const targetSession = sessionName || this.sessionName; - const result = await this.executeTmuxCommand(['list-windows', '-t', targetSession]); + const result = await this.executeTmuxCommand(['list-windows', '-t', targetSession, '-F', '#W']); if (!result || result.returncode !== 0) { return []; } - // Parse window names from tmux output - const windows: string[] = []; - const lines = result.stdout.trim().split('\n'); - - for (const line of lines) { - const match = line.match(/^\d+:\s+(\w+)/); - if (match) { - windows.push(match[1]); - } - } - - return windows; + return result.stdout + .split('\n') + .map((line) => line.trim()) + .filter(Boolean); } } @@ -964,24 +989,25 @@ export async function createTmuxSession( } ): Promise<{ success: boolean; sessionIdentifier?: string; error?: string }> { try { - if (!sessionName || !/^[a-zA-Z0-9._-]+$/.test(sessionName)) { + const trimmedSessionName = sessionName?.trim(); + if (!trimmedSessionName || !/^[a-zA-Z0-9._ -]+$/.test(trimmedSessionName)) { throw new TmuxSessionIdentifierError(`Invalid session name: "${sessionName}"`); } - const utils = new TmuxUtilities(sessionName); + const utils = new TmuxUtilities(trimmedSessionName); const windowName = options?.windowName || 'main'; const cmd = ['new-session']; if (options?.detached !== false) { cmd.push('-d'); } - cmd.push('-s', sessionName); + cmd.push('-s', trimmedSessionName); cmd.push('-n', windowName); const result = await utils.executeTmuxCommand(cmd); if (result && result.returncode === 0) { const sessionIdentifier: TmuxSessionIdentifier = { - session: sessionName, + session: trimmedSessionName, window: windowName }; return { @@ -1026,11 +1052,11 @@ export function buildTmuxSessionIdentifier(params: { pane?: string; }): { success: boolean; identifier?: string; error?: string } { try { - if (!params.session || !/^[a-zA-Z0-9._-]+$/.test(params.session)) { + if (!params.session || !/^[a-zA-Z0-9._ -]+$/.test(params.session)) { throw new TmuxSessionIdentifierError(`Invalid session name: "${params.session}"`); } - if (params.window && !/^[a-zA-Z0-9._-]+$/.test(params.window)) { + if (params.window && !/^[a-zA-Z0-9._ -]+$/.test(params.window)) { throw new TmuxSessionIdentifierError(`Invalid window name: "${params.window}"`); } @@ -1049,4 +1075,4 @@ export function buildTmuxSessionIdentifier(params: { error: error instanceof Error ? error.message : 'Unknown error' }; } -} \ No newline at end of file +}