From 4616f7ab1cdaf95449b21c7ed3f9f316be41120a Mon Sep 17 00:00:00 2001 From: Ben Younes Date: Fri, 13 Mar 2026 03:58:00 +0100 Subject: [PATCH] fix: output valid JSON from smart-install.js hook to prevent SessionStart error (#1337) smart-install.js used stdio: 'inherit' for execSync calls, leaking plain-text install output (bun/npm progress) to stdout. Claude Code expects hook output to start with '{' (valid JSON), so this caused a confusing "SessionStart:startup hook error" on every session start. Changes: - Pipe stdout on all execSync calls to prevent non-JSON stdout leak - Output {"continue":true,"suppressOutput":true} on both success and error paths so Claude Code always receives valid JSON - Add tests verifying no stdio:'inherit' remains and JSON is output Fixes #1253 Vibe-coded by Ousama Ben Younes Co-authored-by: Claude Opus 4.6 --- plugin/scripts/smart-install.js | 27 ++++++++---- tests/smart-install.test.ts | 74 +++++++++++++++++++++++++++++++++ 2 files changed, 92 insertions(+), 9 deletions(-) diff --git a/plugin/scripts/smart-install.js b/plugin/scripts/smart-install.js index c5c265d4..64df272b 100644 --- a/plugin/scripts/smart-install.js +++ b/plugin/scripts/smart-install.js @@ -220,14 +220,14 @@ function installBun() { // Windows: Use PowerShell installer console.error(' Installing via PowerShell...'); execSync('powershell -c "irm bun.sh/install.ps1 | iex"', { - stdio: 'inherit', + stdio: ['pipe', 'pipe', 'inherit'], shell: true }); } else { // Unix/macOS: Use curl installer console.error(' Installing via curl...'); execSync('curl -fsSL https://bun.sh/install | bash', { - stdio: 'inherit', + stdio: ['pipe', 'pipe', 'inherit'], shell: true }); } @@ -285,14 +285,14 @@ function installUv() { // Windows: Use PowerShell installer console.error(' Installing via PowerShell...'); execSync('powershell -ExecutionPolicy ByPass -c "irm https://astral.sh/uv/install.ps1 | iex"', { - stdio: 'inherit', + stdio: ['pipe', 'pipe', 'inherit'], shell: true }); } else { // Unix/macOS: Use curl installer console.error(' Installing via curl...'); execSync('curl -LsSf https://astral.sh/uv/install.sh | sh', { - stdio: 'inherit', + stdio: ['pipe', 'pipe', 'inherit'], shell: true }); } @@ -426,14 +426,18 @@ function installDeps() { // Quote path for Windows paths with spaces const bunCmd = IS_WINDOWS && bunPath.includes(' ') ? `"${bunPath}"` : bunPath; + // Use pipe for stdout to prevent non-JSON output leaking to Claude Code hooks. + // stderr is inherited so progress/errors are still visible to the user. + const installStdio = ['pipe', 'pipe', 'inherit']; + let bunSucceeded = false; try { - execSync(`${bunCmd} install`, { cwd: ROOT, stdio: 'inherit', shell: IS_WINDOWS }); + execSync(`${bunCmd} install`, { cwd: ROOT, stdio: installStdio, shell: IS_WINDOWS }); bunSucceeded = true; } catch { // First attempt failed, try with force flag try { - execSync(`${bunCmd} install --force`, { cwd: ROOT, stdio: 'inherit', shell: IS_WINDOWS }); + execSync(`${bunCmd} install --force`, { cwd: ROOT, stdio: installStdio, shell: IS_WINDOWS }); bunSucceeded = true; } catch { // Bun failed completely, will try npm fallback @@ -445,7 +449,7 @@ function installDeps() { console.error('⚠️ Bun install failed, falling back to npm...'); console.error(' (This can happen with npm alias packages like *-cjs)'); try { - execSync('npm install', { cwd: ROOT, stdio: 'inherit', shell: IS_WINDOWS }); + execSync('npm install', { cwd: ROOT, stdio: installStdio, shell: IS_WINDOWS }); } catch (npmError) { throw new Error('Both bun and npm install failed: ' + npmError.message); } @@ -506,7 +510,7 @@ try { console.error(`⚠️ Bun ${currentVersion} is outdated. Minimum required: ${MIN_BUN_VERSION}`); console.error(' Upgrading bun...'); try { - execSync('bun upgrade', { stdio: 'inherit', shell: IS_WINDOWS }); + execSync('bun upgrade', { stdio: ['pipe', 'pipe', 'inherit'], shell: IS_WINDOWS }); if (!isBunVersionSufficient()) { console.error(`❌ Bun upgrade failed. Please manually upgrade: bun upgrade`); process.exit(1); @@ -542,7 +546,7 @@ try { if (!verifyCriticalModules()) { console.error('⚠️ Retrying install with npm...'); try { - execSync('npm install --production', { cwd: ROOT, stdio: 'inherit', shell: IS_WINDOWS }); + execSync('npm install --production', { cwd: ROOT, stdio: ['pipe', 'pipe', 'inherit'], shell: IS_WINDOWS }); } catch { // npm also failed } @@ -577,7 +581,12 @@ try { // Step 4: Install CLI to PATH installCLI(); + + // Output valid JSON for Claude Code hook contract + console.log(JSON.stringify({ continue: true, suppressOutput: true })); } catch (e) { console.error('❌ Installation failed:', e.message); + // Still output valid JSON so Claude Code doesn't show a confusing error + console.log(JSON.stringify({ continue: true, suppressOutput: true })); process.exit(1); } diff --git a/tests/smart-install.test.ts b/tests/smart-install.test.ts index 7c3f5f6d..2a79557f 100644 --- a/tests/smart-install.test.ts +++ b/tests/smart-install.test.ts @@ -2,6 +2,7 @@ import { describe, it, expect, beforeEach, afterEach } from 'bun:test'; import { existsSync, mkdirSync, writeFileSync, rmSync, readFileSync } from 'fs'; import { join } from 'path'; import { tmpdir } from 'os'; +import { spawnSync } from 'child_process'; /** * Smart Install Script Tests @@ -163,3 +164,76 @@ describe('smart-install verifyCriticalModules logic', () => { expect(missing).toEqual(['@chroma-core/other-pkg']); }); }); + +describe('smart-install stdout JSON output (#1253)', () => { + const SCRIPT_PATH = join(__dirname, '..', 'plugin', 'scripts', 'smart-install.js'); + + it('should not have any execSync with stdio: inherit (prevents stdout leak)', () => { + const content = readFileSync(SCRIPT_PATH, 'utf-8'); + // stdio: 'inherit' would leak non-JSON output to stdout, breaking Claude Code hooks + expect(content).not.toContain("stdio: 'inherit'"); + expect(content).not.toContain('stdio: "inherit"'); + }); + + it('should output valid JSON to stdout on success path', () => { + const content = readFileSync(SCRIPT_PATH, 'utf-8'); + // The script must print JSON to stdout for the Claude Code hook contract + expect(content).toContain('console.log(JSON.stringify('); + expect(content).toContain('continue'); + expect(content).toContain('suppressOutput'); + }); + + it('should output valid JSON to stdout even in error catch block', () => { + const content = readFileSync(SCRIPT_PATH, 'utf-8'); + // Find the catch block and verify it also outputs JSON + const catchIndex = content.lastIndexOf('catch (e)'); + expect(catchIndex).toBeGreaterThan(0); + const catchBlock = content.slice(catchIndex, catchIndex + 300); + expect(catchBlock).toContain('console.log(JSON.stringify('); + }); + + it('should use piped stdout for all execSync calls', () => { + const content = readFileSync(SCRIPT_PATH, 'utf-8'); + // All execSync calls should pipe stdout to prevent leaking to the hook output. + // Match execSync calls that have a stdio option — they should all use array form. + // All execSync calls should either use 'ignore', array form, or the installStdio variable + // — never bare 'inherit' which leaks non-JSON output to stdout + expect(content).not.toContain("stdio: 'inherit'"); + expect(content).not.toContain('stdio: "inherit"'); + // Verify the installStdio variable is defined with the correct pipe config + expect(content).toContain("const installStdio = ['pipe', 'pipe', 'inherit']"); + }); + + it('should produce valid JSON when run with plugin disabled', () => { + // Run the actual script with the plugin forcefully disabled via settings + // This exercises the early exit path + const settingsDir = join(tmpdir(), `claude-mem-test-settings-${process.pid}`); + const settingsFile = join(settingsDir, 'settings.json'); + mkdirSync(settingsDir, { recursive: true }); + writeFileSync(settingsFile, JSON.stringify({ + enabledPlugins: { 'claude-mem@thedotmack': false } + })); + + try { + const result = spawnSync('node', [SCRIPT_PATH], { + encoding: 'utf-8', + env: { + ...process.env, + CLAUDE_CONFIG_DIR: settingsDir, + }, + timeout: 10000, + }); + + // When plugin is disabled, script exits with 0 and produces no stdout + // (the early exit at line 31-33 calls process.exit(0) before any output) + expect(result.status).toBe(0); + // stdout should be empty or valid JSON (not plain text install messages) + const stdout = (result.stdout || '').trim(); + if (stdout.length > 0) { + expect(() => JSON.parse(stdout)).not.toThrow(); + } + } finally { + rmSync(settingsDir, { recursive: true, force: true }); + } + }); +});