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 <noreply@anthropic.com>
This commit is contained in:
@@ -220,14 +220,14 @@ function installBun() {
|
|||||||
// Windows: Use PowerShell installer
|
// Windows: Use PowerShell installer
|
||||||
console.error(' Installing via PowerShell...');
|
console.error(' Installing via PowerShell...');
|
||||||
execSync('powershell -c "irm bun.sh/install.ps1 | iex"', {
|
execSync('powershell -c "irm bun.sh/install.ps1 | iex"', {
|
||||||
stdio: 'inherit',
|
stdio: ['pipe', 'pipe', 'inherit'],
|
||||||
shell: true
|
shell: true
|
||||||
});
|
});
|
||||||
} else {
|
} else {
|
||||||
// Unix/macOS: Use curl installer
|
// Unix/macOS: Use curl installer
|
||||||
console.error(' Installing via curl...');
|
console.error(' Installing via curl...');
|
||||||
execSync('curl -fsSL https://bun.sh/install | bash', {
|
execSync('curl -fsSL https://bun.sh/install | bash', {
|
||||||
stdio: 'inherit',
|
stdio: ['pipe', 'pipe', 'inherit'],
|
||||||
shell: true
|
shell: true
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
@@ -285,14 +285,14 @@ function installUv() {
|
|||||||
// Windows: Use PowerShell installer
|
// Windows: Use PowerShell installer
|
||||||
console.error(' Installing via PowerShell...');
|
console.error(' Installing via PowerShell...');
|
||||||
execSync('powershell -ExecutionPolicy ByPass -c "irm https://astral.sh/uv/install.ps1 | iex"', {
|
execSync('powershell -ExecutionPolicy ByPass -c "irm https://astral.sh/uv/install.ps1 | iex"', {
|
||||||
stdio: 'inherit',
|
stdio: ['pipe', 'pipe', 'inherit'],
|
||||||
shell: true
|
shell: true
|
||||||
});
|
});
|
||||||
} else {
|
} else {
|
||||||
// Unix/macOS: Use curl installer
|
// Unix/macOS: Use curl installer
|
||||||
console.error(' Installing via curl...');
|
console.error(' Installing via curl...');
|
||||||
execSync('curl -LsSf https://astral.sh/uv/install.sh | sh', {
|
execSync('curl -LsSf https://astral.sh/uv/install.sh | sh', {
|
||||||
stdio: 'inherit',
|
stdio: ['pipe', 'pipe', 'inherit'],
|
||||||
shell: true
|
shell: true
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
@@ -426,14 +426,18 @@ function installDeps() {
|
|||||||
// Quote path for Windows paths with spaces
|
// Quote path for Windows paths with spaces
|
||||||
const bunCmd = IS_WINDOWS && bunPath.includes(' ') ? `"${bunPath}"` : bunPath;
|
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;
|
let bunSucceeded = false;
|
||||||
try {
|
try {
|
||||||
execSync(`${bunCmd} install`, { cwd: ROOT, stdio: 'inherit', shell: IS_WINDOWS });
|
execSync(`${bunCmd} install`, { cwd: ROOT, stdio: installStdio, shell: IS_WINDOWS });
|
||||||
bunSucceeded = true;
|
bunSucceeded = true;
|
||||||
} catch {
|
} catch {
|
||||||
// First attempt failed, try with force flag
|
// First attempt failed, try with force flag
|
||||||
try {
|
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;
|
bunSucceeded = true;
|
||||||
} catch {
|
} catch {
|
||||||
// Bun failed completely, will try npm fallback
|
// Bun failed completely, will try npm fallback
|
||||||
@@ -445,7 +449,7 @@ function installDeps() {
|
|||||||
console.error('⚠️ Bun install failed, falling back to npm...');
|
console.error('⚠️ Bun install failed, falling back to npm...');
|
||||||
console.error(' (This can happen with npm alias packages like *-cjs)');
|
console.error(' (This can happen with npm alias packages like *-cjs)');
|
||||||
try {
|
try {
|
||||||
execSync('npm install', { cwd: ROOT, stdio: 'inherit', shell: IS_WINDOWS });
|
execSync('npm install', { cwd: ROOT, stdio: installStdio, shell: IS_WINDOWS });
|
||||||
} catch (npmError) {
|
} catch (npmError) {
|
||||||
throw new Error('Both bun and npm install failed: ' + npmError.message);
|
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(`⚠️ Bun ${currentVersion} is outdated. Minimum required: ${MIN_BUN_VERSION}`);
|
||||||
console.error(' Upgrading bun...');
|
console.error(' Upgrading bun...');
|
||||||
try {
|
try {
|
||||||
execSync('bun upgrade', { stdio: 'inherit', shell: IS_WINDOWS });
|
execSync('bun upgrade', { stdio: ['pipe', 'pipe', 'inherit'], shell: IS_WINDOWS });
|
||||||
if (!isBunVersionSufficient()) {
|
if (!isBunVersionSufficient()) {
|
||||||
console.error(`❌ Bun upgrade failed. Please manually upgrade: bun upgrade`);
|
console.error(`❌ Bun upgrade failed. Please manually upgrade: bun upgrade`);
|
||||||
process.exit(1);
|
process.exit(1);
|
||||||
@@ -542,7 +546,7 @@ try {
|
|||||||
if (!verifyCriticalModules()) {
|
if (!verifyCriticalModules()) {
|
||||||
console.error('⚠️ Retrying install with npm...');
|
console.error('⚠️ Retrying install with npm...');
|
||||||
try {
|
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 {
|
} catch {
|
||||||
// npm also failed
|
// npm also failed
|
||||||
}
|
}
|
||||||
@@ -577,7 +581,12 @@ try {
|
|||||||
|
|
||||||
// Step 4: Install CLI to PATH
|
// Step 4: Install CLI to PATH
|
||||||
installCLI();
|
installCLI();
|
||||||
|
|
||||||
|
// Output valid JSON for Claude Code hook contract
|
||||||
|
console.log(JSON.stringify({ continue: true, suppressOutput: true }));
|
||||||
} catch (e) {
|
} catch (e) {
|
||||||
console.error('❌ Installation failed:', e.message);
|
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);
|
process.exit(1);
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -2,6 +2,7 @@ import { describe, it, expect, beforeEach, afterEach } from 'bun:test';
|
|||||||
import { existsSync, mkdirSync, writeFileSync, rmSync, readFileSync } from 'fs';
|
import { existsSync, mkdirSync, writeFileSync, rmSync, readFileSync } from 'fs';
|
||||||
import { join } from 'path';
|
import { join } from 'path';
|
||||||
import { tmpdir } from 'os';
|
import { tmpdir } from 'os';
|
||||||
|
import { spawnSync } from 'child_process';
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Smart Install Script Tests
|
* Smart Install Script Tests
|
||||||
@@ -163,3 +164,76 @@ describe('smart-install verifyCriticalModules logic', () => {
|
|||||||
expect(missing).toEqual(['@chroma-core/other-pkg']);
|
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 });
|
||||||
|
}
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
Reference in New Issue
Block a user