Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
2 changes: 2 additions & 0 deletions apps/daemon/src/agent-protocol/acp/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,5 +37,7 @@ export const ACP_ARTIFACT_ECHO_START_RE = new RegExp(
export const ACP_RAW_EVENT_SHAPE_DIAGNOSTIC_LIMIT = 8;
/** Maximum number of bytes retained from stderr to detect AMR retry/failure signals; older bytes are discarded to bound memory use. */
export const AMR_STDERR_RETRY_TAIL_LIMIT = 16_000;
/** Maximum number of redacted stderr characters attached to an ACP child-exit diagnostic. */
export const ACP_STDERR_DIAGNOSTIC_TAIL_LIMIT = 4_000;
/** Normalised token IDs that identify a model-selection config option in an ACP `session/new` response's `configOptions` array. */
export const MODEL_CONFIG_OPTION_IDS = new Set(['model', 'models', 'modelid', 'modelids']);
34 changes: 29 additions & 5 deletions apps/daemon/src/agent-protocol/acp/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
createToolCallTextSuppressor,
type ArtifactTextSuppressor,
} from '../../artifacts/text-suppression.js';
import { redactSecrets } from '../../redact.js';
import { createJsonLineStream } from '../core/index.js';
import type { JsonRpcId, JsonObject, TimerHandle, AcpChildProcess } from './types.js';
import {
Expand All @@ -21,6 +22,7 @@ import {
ACP_ARTIFACT_ECHO_START_RE,
ACP_RAW_EVENT_SHAPE_DIAGNOSTIC_LIMIT,
AMR_STDERR_RETRY_TAIL_LIMIT,
ACP_STDERR_DIAGNOSTIC_TAIL_LIMIT,
} from './constants.js';
import { errorMessage, asObject, extractAcpUpdateText } from './json.js';
import {
Expand Down Expand Up @@ -168,7 +170,8 @@ export function attachAcpSession({
let emittedTextBuffer = '';
let rawAcpShapeDiagnosticCount = 0;
let artifactSuppressionDiagnosticCount = 0;
let amrStderrRetryTail = '';
let acpStderrTail = '';
let currentStage = 'initialize';
let finished = false;
let fatal = false;
let aborted = false;
Expand Down Expand Up @@ -354,6 +357,7 @@ export function attachAcpSession({
};

const writeRpc = (id: JsonRpcId, method: string, params: unknown, timeoutLabel: string) => {
currentStage = timeoutLabel;
resetStageTimer(timeoutLabel);
try {
sendRpc(stdin, id, method, params);
Expand Down Expand Up @@ -928,18 +932,38 @@ export function attachAcpSession({
stdout.on('data', (chunk: string) => parser.feed(chunk));
child.stderr?.setEncoding('utf8');
child.stderr?.on('data', (chunk: string) => {
if (!modelUnavailableErrorCode || finished) return;
amrStderrRetryTail = `${amrStderrRetryTail}${String(chunk)}`.slice(
if (finished) return;
acpStderrTail = `${acpStderrTail}${String(chunk)}`.slice(
-AMR_STDERR_RETRY_TAIL_LIMIT,
);
const promotedPayload = promotedAmrStderrPayload(amrStderrRetryTail);
if (!modelUnavailableErrorCode) return;
const promotedPayload = promotedAmrStderrPayload(acpStderrTail);
if (promotedPayload) failWithPayload(promotedPayload);
});
child.on('close', (code, signal) => {
clearStageTimer();
parser.flush();
if (!finished && !aborted && !fatal) {
fail(`ACP session exited before completion (code=${code ?? 'null'}, signal=${signal ?? 'none'})`);
const stderrTail = redactSecrets(
acpStderrTail
.replace(/\u001B\[[0-?]*[ -/]*[@-~]/gu, '')
.replace(/\r\n?/gu, '\n')
.replace(/[\u0000-\u0008\u000B\u000C\u000E-\u001F\u007F]/gu, ''),
)
.trim()
.slice(-ACP_STDERR_DIAGNOSTIC_TAIL_LIMIT);
fail(
`ACP session exited before completion (code=${code ?? 'null'}, signal=${signal ?? 'none'})`,
{
details: {
kind: 'acp_child_exit',
phase: currentStage,
exit_code: code,
signal,
...(stderrTail ? { stderr_tail: stderrTail } : {}),
},
},
);
}
});
child.on('error', (err: Error) => fail(err.message));
Expand Down
79 changes: 79 additions & 0 deletions apps/daemon/tests/acp.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2059,6 +2059,85 @@ test('attachAcpSession does not double-kill a child that exits cleanly on stdin.
}
});

test('attachAcpSession preserves redacted stderr diagnostics for startup exits', () => {
const child = new FakeAcpChild();
const events: Array<{ event: string; payload: unknown }> = [];
const apiKey = `sk-test-${'a'.repeat(24)}`;

const session = attachAcpSession({
child: child as never,
prompt: 'hello',
cwd: '/tmp/od-project',
model: null,
mcpServers: [],
send: (event, payload) => events.push({ event, payload }),
});

child.stderr.write(`\u001b[31mHermes startup failed\u001b[0m\nAPI key: ${apiKey}\n`);
child.emit('close', 1, null);

assert.equal(session.hasFatalError(), true);
const error = events.find((entry) => entry.event === 'error')?.payload as {
error?: { details?: Record<string, unknown> };
};
assert.deepEqual(error.error?.details, {
kind: 'acp_child_exit',
phase: 'initialize',
exit_code: 1,
signal: null,
stderr_tail: 'Hermes startup failed\nAPI key: [REDACTED:sk_key]',
});
assert.equal(JSON.stringify(error).includes(apiKey), false);
});

test('attachAcpSession redacts stderr credentials split by ANSI sequences', () => {
const child = new FakeAcpChild();
const events: Array<{ event: string; payload: unknown }> = [];
const apiKey = `sk-test-${'a'.repeat(12)}${'b'.repeat(12)}`;
const ansiSplitApiKey = `sk-test-${'a'.repeat(12)}\u001b[31m${'b'.repeat(12)}\u001b[0m`;

attachAcpSession({
child: child as never,
prompt: 'hello',
cwd: '/tmp/od-project',
model: null,
mcpServers: [],
send: (event, payload) => events.push({ event, payload }),
});

child.stderr.write(`API key: ${ansiSplitApiKey}\n`);
child.emit('close', 1, null);

const error = events.find((entry) => entry.event === 'error')?.payload as {
error?: { details?: Record<string, unknown> };
};
assert.equal(error.error?.details?.stderr_tail, 'API key: [REDACTED:sk_key]');
assert.equal(JSON.stringify(error).includes(apiKey), false);
});

test('attachAcpSession classifies exits after initialize as session setup failures', () => {
const child = new FakeAcpChild();
const events: Array<{ event: string; payload: unknown }> = [];

attachAcpSession({
child: child as never,
prompt: 'hello',
cwd: '/tmp/od-project',
model: null,
mcpServers: [],
send: (event, payload) => events.push({ event, payload }),
});

writeAcpResult(child, 1, {});
child.stderr.write('Kimi could not create a session\n');
child.emit('close', 1, null);

const error = events.find((entry) => entry.event === 'error')?.payload as {
error?: { details?: Record<string, unknown> };
};
assert.equal(error.error?.details?.phase, 'session/new');
});

test('attachAcpSession accepts an opted-in ACP turn_end update as prompt completion', () => {
const child = new FakeAcpChild();
const events: Array<{ event: string; payload: unknown }> = [];
Expand Down
Loading