From c4f7a15d295fa16ab7dbd3a72e144967908d2ae6 Mon Sep 17 00:00:00 2001 From: kina0630 Date: Thu, 8 Oct 2026 21:52:39 +0800 Subject: [PATCH] fix: forward app-server stderr to adapter stderr Forward app-server stderr buffers unchanged to adapter stderr while the child is running, independent of APP_SERVER_LOGS. Keep the optional file copy, JSON-RPC stdout handling, and crash-time stderr capture. Ignore EPIPE and ERR_STREAM_DESTROYED on adapter stderr so diagnostics written after the client closes the stderr reader do not terminate the adapter; other errors still propagate. Assisted-by: GPT-6 Astra --- README.md | 4 +- readme-dev.md | 4 +- src/CodexJsonRpcConnection.ts | 10 +- src/__tests__/CodexJsonRpcConnection.test.ts | 157 +++++++++++++++++++ 4 files changed, 172 insertions(+), 3 deletions(-) create mode 100644 src/__tests__/CodexJsonRpcConnection.test.ts diff --git a/README.md b/README.md index 0c4ba667..0e948b11 100644 --- a/README.md +++ b/README.md @@ -73,7 +73,9 @@ The adapter advertises ACP auth methods during initialization. Clients can authe - `DEFAULT_AUTH_REQUEST` - ACP auth request JSON used when Codex requires authentication. - `INITIAL_AGENT_MODE` - initial mode id: `read-only`, `workspace-write`, `agent`, or `agent-full-access`. - `NO_BROWSER` - hide browser-based ChatGPT auth when set. -- `APP_SERVER_LOGS` - directory for adapter logs. +- `APP_SERVER_LOGS` - directory for adapter logs. When set, app-server stderr is also copied to `app-server.log`. + +App-server stderr is always forwarded unchanged to adapter stderr. ## Development diff --git a/readme-dev.md b/readme-dev.md index 458c263f..eb0763ac 100644 --- a/readme-dev.md +++ b/readme-dev.md @@ -11,10 +11,12 @@ Set `CODEX_PATH` to run a different Codex binary; versions other than the one sp - `DEFAULT_AUTH_REQUEST` - ACP auth request JSON used when Codex requires authentication. - `INITIAL_AGENT_MODE` - initial mode id: `read-only`, `workspace-write`, `agent`, or `agent-full-access`. - `NO_BROWSER` - hide browser-based ChatGPT auth when set. -- `APP_SERVER_LOGS` - directory for adapter logs. +- `APP_SERVER_LOGS` - directory for adapter logs. When set, app-server stderr is also copied to `app-server.log`. - `CODEX_ACP_APP_SERVER_CRASH_LIMIT` - how many crashes of the Codex app-server in the crash window stop its automatic restart (default `5`). - `CODEX_ACP_APP_SERVER_CRASH_WINDOW_MS` - the crash window in milliseconds (default `300000`, 5 minutes). +App-server stderr is always forwarded unchanged to adapter stderr. + ### Quick start #### Develop on Windows? diff --git a/src/CodexJsonRpcConnection.ts b/src/CodexJsonRpcConnection.ts index 94fb23bb..40da48b0 100644 --- a/src/CodexJsonRpcConnection.ts +++ b/src/CodexJsonRpcConnection.ts @@ -10,6 +10,13 @@ import {logger} from "./Logger"; const require = createRequire(import.meta.url); +// Register once for all connections, including app-server restarts. +process.stderr.on("error", (error: NodeJS.ErrnoException) => { + if (error.code !== "EPIPE" && error.code !== "ERR_STREAM_DESTROYED") { + throw error; + } +}); + export interface CodexConnection { readonly connection: MessageConnection readonly process: ChildProcessWithoutNullStreams; @@ -91,8 +98,9 @@ export function attachLogs(proc: ChildProcessWithoutNullStreams) { return originalWrite(chunk, encoding, callback); }; - proc.stderr.on("data", (data) => { + proc.stderr.on("data", (data: Buffer) => { if (logger.enabled) logger.log(`[ERR] ${data.toString()}`); + process.stderr.write(data); }); proc.stdout.on("data", (data: Buffer) => { if (logger.enabled) logger.log(`[OUT] ${data.toString()}`); diff --git a/src/__tests__/CodexJsonRpcConnection.test.ts b/src/__tests__/CodexJsonRpcConnection.test.ts new file mode 100644 index 00000000..37f3a4ac --- /dev/null +++ b/src/__tests__/CodexJsonRpcConnection.test.ts @@ -0,0 +1,157 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { spawn } from 'node:child_process'; +import { once } from 'node:events'; +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import type { CodexConnection } from '../CodexJsonRpcConnection'; + +vi.mock('node:child_process', async (importOriginal) => { + const original = await importOriginal(); + return { ...original, spawn: vi.fn() }; +}); + +// A real, shell-independent child keeps stdout reserved for JSON-RPC responses. +const fixture = ` + const readline = require('node:readline'); + readline.createInterface({ input: process.stdin }).on('line', (line) => { + const request = JSON.parse(line); + process.stderr.write(Buffer.from(request.params.bytes), () => { + process.stdout.write(JSON.stringify({ id: request.id, result: 'ok' }) + '\\n'); + }); + }); +`; + +let codex: CodexConnection | undefined; +let closed: Promise | undefined; +let tempDir: string; +let stderrListeners: ReturnType; + +beforeEach(() => { + tempDir = fs.mkdtempSync(path.join(os.tmpdir(), 'codex-stderr-')); + stderrListeners = process.stderr.listeners('error'); +}); + +afterEach(async () => { + try { + if (codex) { + codex.connection.dispose(); + codex.process.kill(); + await closed; + } + } finally { + codex = undefined; + closed = undefined; + vi.restoreAllMocks(); + vi.unstubAllEnvs(); + vi.resetModules(); + for (const listener of process.stderr.listeners('error')) { + if (!stderrListeners.includes(listener)) process.stderr.removeListener('error', listener); + } + fs.rmSync(tempDir, { recursive: true, force: true }); + } +}); + +describe('CodexJsonRpcConnection', () => { + it.each([ + { name: 'without file logging', fileLogs: false }, + { name: 'with file logging', fileLogs: true }, + ])('forwards live stderr $name', async ({ fileLogs }) => { + const logDir = path.join(tempDir, 'logs'); + vi.stubEnv('APP_SERVER_LOGS', fileLogs ? logDir : undefined); + // Logger reads APP_SERVER_LOGS when its module-level singleton is constructed. + vi.resetModules(); + const original = await vi.importActual('node:child_process'); + vi.mocked(spawn).mockImplementation(() => original.spawn(process.execPath, ['-e', fixture])); + const { startCodexConnection } = await import('../CodexJsonRpcConnection'); + const stderr = vi.spyOn(process.stderr, 'write').mockImplementation(() => true); + const stdout = vi.spyOn(process.stdout, 'write').mockImplementation(() => true); + codex = startCodexConnection(); + closed = once(codex.process, 'close'); + + const marker = Buffer.from('app-server diagnostic: 警告\n'); + await expect(codex.connection.sendRequest('diagnostic', { bytes: [...marker] })).resolves.toBe('ok'); + await vi.waitFor(() => { + expect(Buffer.concat(stderr.mock.calls.map(([chunk]) => { + expect(Buffer.isBuffer(chunk)).toBe(true); + return chunk as Buffer; + }))).toEqual(marker); + }); + expect(codex.process.exitCode).toBeNull(); + expect(codex.process.signalCode).toBeNull(); + expect(stdout).not.toHaveBeenCalled(); + if (fileLogs) { + const log = fs.readFileSync(path.join(logDir, 'app-server.log'), 'utf8'); + expect(log).toContain('[ERR]'); + expect(log).toContain('app-server diagnostic:'); + } else { + expect(process.env['APP_SERVER_LOGS']).toBeUndefined(); + expect(fs.existsSync(logDir)).toBe(false); + expect(fs.readdirSync(tempDir)).toEqual([]); + } + + // Wait for the first byte to arrive before sending the rest of a UTF-8 character. + // This forces a chunk boundary without depending on OS pipe coalescing or timers. + stderr.mockClear(); + const unicode = Buffer.from('界\n'); + for (const bytes of [unicode.subarray(0, 1), unicode.subarray(1)]) { + const count = stderr.mock.calls.length; + await expect(codex.connection.sendRequest('diagnostic', { bytes: [...bytes] })).resolves.toBe('ok'); + await vi.waitFor(() => expect(stderr.mock.calls.length).toBeGreaterThan(count)); + } + expect(stderr.mock.calls.every(([chunk]) => Buffer.isBuffer(chunk))).toBe(true); + expect(Buffer.concat(stderr.mock.calls.map(([chunk]) => chunk as Buffer))).toEqual(unicode); + expect(stdout).not.toHaveBeenCalled(); + }, 15_000); + + // The executable fixture uses a POSIX shebang and chmod. + it.skipIf(process.platform === 'win32')('keeps serving JSON-RPC after the stderr reader closes', async () => { + const fixturePath = path.join(tempDir, 'app-server.cjs'); + fs.writeFileSync(fixturePath, `#!${process.execPath}\n${fixture}`); + fs.chmodSync(fixturePath, 0o755); + const moduleUrl = new URL('../CodexJsonRpcConnection.ts', import.meta.url).href; + const harness = ` + import { once } from 'node:events'; + import { startCodexConnection } from ${JSON.stringify(moduleUrl)}; + const codex = startCodexConnection(${JSON.stringify(fixturePath)}); + const closed = once(codex.process, 'close'); + try { + const trigger = once(process.stdin, 'data'); + process.stdout.write('ready\\n'); + await trigger; + process.stdin.destroy(); + const diagnostic = once(codex.process.stderr, 'data'); + const result = await codex.connection.sendRequest('diagnostic', { + bytes: [...Buffer.from('diagnostic after disconnect\\n')], + }); + await diagnostic; + // Allow the stderr write's asynchronous error to fire before completing. + await new Promise(setImmediate); + process.stdout.write(result + '\\n'); + } finally { + codex.connection.dispose(); + codex.process.kill(); + await closed; + } + `; + const original = await vi.importActual('node:child_process'); + const child = original.spawn(process.execPath, ['--import', 'tsx', '--input-type=module', '-e', harness], { + env: { ...process.env, APP_SERVER_LOGS: '' }, + }); + const childClosed = once(child, 'close'); + let output = ''; + child.stdout.on('data', (chunk: Buffer) => { output += chunk.toString(); }); + try { + await vi.waitFor(() => expect(output).toBe('ready\n'), { timeout: 10_000 }); + const stderrClosed = once(child.stderr, 'close'); + child.stderr.destroy(); + await stderrClosed; + child.stdin.end('write diagnostic\n'); + expect(await childClosed).toEqual([0, null]); + expect(output).toBe('ready\nok\n'); + } finally { + if (child.exitCode === null && child.signalCode === null) child.kill(); + await childClosed; + } + }, 15_000); +});