Skip to content
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 47 additions & 12 deletions libs/platform/src/subprocess.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,31 @@
/**
* Subprocess management utilities for CLI providers
*
* JSONL parsing is tolerant of non-JSON output (e.g., from bun/npm install).
* Lines that don't start with { or [ are logged but not treated as errors.
*/

import { spawn, type ChildProcess } from 'child_process';
import { StringDecoder } from 'string_decoder';

/**
* Checks if a line looks like a JSON candidate (starts with { or [)
* This is used to tolerate non-JSON output from tools like bun/npm install
*/
function isJsonCandidate(line: string): boolean {
if (!line) return false;
const firstChar = line[0];
return firstChar === '{' || firstChar === '[';
}

/**
* Logs a non-JSON line that was skipped during parsing
*/
function logNonJsonOutput(trimmed: string, isFinal: boolean = false): void {
const suffix = isFinal ? ' (final)' : '';
console.log(`[SubprocessManager] Non-JSON output${suffix}: ${trimmed}`);
}

export interface SubprocessOptions {
command: string;
args: string[];
Expand Down Expand Up @@ -191,6 +212,14 @@ export async function* spawnJSONLProcess(options: SubprocessOptions): AsyncGener
const trimmed = line.trim();
if (!trimmed) continue;

// Only try to parse lines that look like JSON (start with { or [)
// This tolerates non-JSON output from tools like bun/npm install
if (!isJsonCandidate(trimmed)) {
// Non-JSON output (e.g., "bun install v1.3.10") - log but don't error
logNonJsonOutput(trimmed);
continue;
}

try {
eventQueue.push(JSON.parse(trimmed));
} catch (parseError) {
Expand All @@ -215,19 +244,25 @@ export async function* spawnJSONLProcess(options: SubprocessOptions): AsyncGener

// Process any remaining partial line
if (lineBuffer.trim()) {
try {
eventQueue.push(JSON.parse(lineBuffer.trim()));
} catch (parseError) {
console.error(
`[SubprocessManager] Failed to parse final JSONL line: ${lineBuffer}`,
parseError
);
eventQueue.push({
type: 'error',
error: `Failed to parse output: ${lineBuffer}`,
});
const trimmed = lineBuffer.trim();

// Only try to parse lines that look like JSON
if (isJsonCandidate(trimmed)) {
try {
eventQueue.push(JSON.parse(trimmed));
} catch (parseError) {
console.error(
`[SubprocessManager] Failed to parse final JSONL line: ${trimmed}`,
parseError
);
eventQueue.push({
type: 'error',
error: `Failed to parse output: ${trimmed}`,
});
}
} else {
logNonJsonOutput(trimmed, true);
}
lineBuffer = '';
}

streamEnded = true;
Expand Down
72 changes: 72 additions & 0 deletions libs/platform/tests/subprocess.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,78 @@ describe('subprocess.ts', () => {
expect(results[1]).toEqual({ type: 'second' });
});

it('should skip non-JSON output lines (e.g., from bun/npm install)', async () => {
const mockProcess = createMockProcess({
stdoutLines: [
'bun install v1.3.10 (30e609e0)',
'{"type":"progress","value":50}',
'Checked 3 installs across 4 packages (no changes) [4.00ms]',
'{"type":"complete"}',
],
exitCode: 0,
});

vi.mocked(cp.spawn).mockReturnValue(mockProcess);

const generator = spawnJSONLProcess(baseOptions);
const results = await collectAsyncGenerator(generator);

// Non-JSON lines should be skipped (logged but not yielded as errors)
expect(results).toHaveLength(2);
expect(results[0]).toEqual({ type: 'progress', value: 50 });
expect(results[1]).toEqual({ type: 'complete' });

// Non-JSON lines should be logged
expect(consoleSpy.log).toHaveBeenCalledWith(
expect.stringContaining('[SubprocessManager] Non-JSON output: bun install')
);
});

it('should log non-JSON output (final) for partial lines at end of stream', async () => {
// Create a custom mock process that ends with a partial non-JSON line (no trailing \n)
const mockProcess = new EventEmitter() as cp.ChildProcess & {
stdout: Readable;
stderr: Readable;
kill: ReturnType<typeof vi.fn>;
};
const stdout = new Readable({ read() {} });
const stderr = new Readable({ read() {} });

mockProcess.stdout = stdout;
mockProcess.stderr = stderr;
mockProcess.kill = vi.fn().mockReturnValue(true);

vi.mocked(cp.spawn).mockReturnValue(mockProcess);

process.nextTick(async () => {
// Push a valid JSON line (with \n)
stdout.push('{"type":"valid"}\n');
await new Promise((resolve) => setImmediate(resolve));

// Push a partial non-JSON line WITHOUT \n - this stays in the buffer
stdout.push('npm WARN something');
await new Promise((resolve) => setImmediate(resolve));

// End stdout - triggers the 'end' handler which flushes the buffer
stdout.push(null);

await new Promise((resolve) => setTimeout(resolve, 10));
mockProcess.emit('exit', 0);
});

const generator = spawnJSONLProcess(baseOptions);
const results = await collectAsyncGenerator(generator);

// Only the valid JSON should be yielded
expect(results).toHaveLength(1);
expect(results[0]).toEqual({ type: 'valid' });

// The final partial non-JSON line should be logged with "(final)" suffix
expect(consoleSpy.log).toHaveBeenCalledWith(
expect.stringContaining('[SubprocessManager] Non-JSON output (final): npm WARN something')
);
});

it('should yield error for malformed JSON and continue processing', async () => {
const mockProcess = createMockProcess({
stdoutLines: ['{"type":"valid"}', '{invalid json}', '{"type":"also_valid"}'],
Expand Down