Skip to content

Commit ef35a08

Browse files
authored
feat(daemon): E-lite root-cause telemetry for inactivity_timeout attribution (#5775)
* feat(daemon): E-lite root-cause telemetry for inactivity_timeout attribution Add four run_finished fields that separate WHY a stalled run died, which the coarse `last_observed_phase` (WHICH phase) cannot: - approval_requested: an ACP approval/permission gate fired. Only the ACP path is daemon-observable (stream/CLI runtimes pass a skip-permissions flag so no gate fires); surfaced via an acp_approval_request diagnostic and folded in by summarizeRunDiagnosticsForAnalytics. - stdin_backpressure: the prompt write to the child's stdin was queued because the OS pipe buffer was full (the child was not draining stdin) — the corroborating signal for a stdin_write-phase stall. - tool_result_sent: a tool_result came back for the run's LAST tool_use. A stall with tool_call_seen && !tool_result_sent is the tool-result-not-delivered root cause. - last_progress_age_ms: age of the last agent activity at finish (the inactivity-watchdog clock). Near the ceiling on a stall, ~0 on a clean finish. Contract fields on RunFinishedProps; daemon populates two off the run object (stdin_backpressure, last_progress_age_ms) and derives two from run.events in the diagnostics summarizer (tool_result_sent, approval_requested). * fix(daemon): pair tool_result_sent by id so parallel tools attribute correctly Review catch (nettee/Looper): the first cut flipped `tool_result_sent` to true on any tool_result after a tool_use, so a multi-tool turn like tool_use(A), tool_use(B), tool_result(A) reported delivered even though B was still outstanding — bucketing multi-tool stalls as "provider stalled after delivery" instead of "tool result not delivered". Now paired by id (`tool_use.id` <-> `tool_result.toolUseId`, the same pairing summarizeRunTimingAnalytics uses): `tool_result_sent` is true only when EVERY committed tool_use received a matching tool_result (outstanding set empty). Tests cover the reviewer's regression case plus parallel all-resolved and sequential-last-outstanding. * fix(daemon): count-pair id-less tool events so degraded stalls aren't "delivered" Second review catch (nettee/Looper): id-based pairing alone still fell through to `tool_result_sent: true` when a runtime emits a tool_use with a null id, because those were counted in `sawAnyToolUse` but never tracked as outstanding. That is not hypothetical — `agent-protocol/pi-rpc/events.ts` and `copilot-stream.ts` both emit `toolCallId ?? null` for tool_use.id AND tool_result.toolUseId on degraded provider events, so an unpaired id-less tool call would mask exactly the stall this metric attributes. Id-less tool events are now paired by count alongside the id-keyed set: `tool_result_sent` requires both an empty outstanding-id set and at least as many id-less results as id-less uses. Tests cover an unpaired id-less stall and an id-less pair that did resolve. * fix(daemon): capture stdin backpressure on the default text-input path Third review catch (nettee/Looper): `stdin_backpressure` never recorded anything for non-`stream-json` runtimes. `child.stdin.end(composed, ...)` returns the stream rather than a boolean, and `writableNeedDrain` is already back to false by the time it returns — even for a chunk that `write(chunk)` would have rejected. Claude is the only runtime on the stream-json path, so the field was permanently false on exactly the runs whose `stdin_write` stalls it exists to attribute. The write and the close are now issued separately through `writePromptAndEndStdin`, which returns the boolean the OS pipe actually gave us. Extracted into chat-run-lifecycle.ts so it is unit-testable: tests cover the backpressured write, the accepted write, and the flush callback still firing (the `stdin_write_end` lifecycle mark). Verified red against the previous behavior.
1 parent c9e622d commit ef35a08

10 files changed

Lines changed: 294 additions & 4 deletions

File tree

apps/daemon/src/agent-protocol/acp/session.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -446,6 +446,16 @@ export function attachAcpSession({
446446
fail(`unhandled ACP permission request: ${JSON.stringify(raw)}`);
447447
return;
448448
}
449+
// E-lite: the ACP path is the only daemon-observable approval gate. Surface
450+
// it so `run_finished.approval_requested` can attribute a `tool_execution`
451+
// stall to an approval hang even though we auto-approve here.
452+
send('agent', {
453+
type: 'diagnostic',
454+
name: 'acp_approval_request',
455+
source: 'acp-json-rpc',
456+
elapsedMs: Date.now() - runStartedAt,
457+
optionId,
458+
});
449459
resetStageTimer('session/request_permission');
450460
try {
451461
sendRpcResult(stdin, raw.id, {

apps/daemon/src/routes/runs.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,11 @@ interface ChatRun {
161161
clients: Set<SseClient>;
162162
analyticsContext?: AnalyticsContext;
163163
analyticsTelemetry?: RunTelemetryTimestamps;
164+
// E-lite root-cause telemetry read at run_finished. `stdinBackpressure`: the
165+
// prompt write to child stdin was queued (pipe buffer full). `lastAgentActivityAt`:
166+
// the inactivity-watchdog clock, used to derive `last_progress_age_ms`.
167+
stdinBackpressure?: boolean;
168+
lastAgentActivityAt?: number;
164169
retryAttemptCount?: number;
165170
retryFinalResult?: string;
166171
retrySuppressedReason?: string;
@@ -1260,6 +1265,12 @@ export function registerRunRoutes(app: Express, ctx: RegisterRunRoutesDeps) {
12601265
} : {}),
12611266
...timingAnalytics,
12621267
...diagnosticsAnalytics,
1268+
// E-lite: `approval_requested`/`tool_result_sent` ride in via
1269+
// `...diagnosticsAnalytics`; these two come off the run object.
1270+
stdin_backpressure: run.stdinBackpressure === true,
1271+
...(typeof run.lastAgentActivityAt === 'number'
1272+
? { last_progress_age_ms: Math.max(0, analyticsCapturedAt - run.lastAgentActivityAt) }
1273+
: {}),
12631274
langfuse_trace_id: run.id,
12641275
...langfuseDeliveryForAnalytics,
12651276
...(errorCode ? { error_code: errorCode } : {}),

apps/daemon/src/run-diagnostics.ts

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,15 @@ export interface RunDiagnosticsAnalytics {
3939
first_token_seen: boolean;
4040
user_visible_output_seen: boolean;
4141
tool_call_seen: boolean;
42+
// True when every committed tool_use received a matching tool_result — paired
43+
// by id where the runtime supplies one, by count for degraded events that emit
44+
// a null id on both sides. A stall with `tool_call_seen && !tool_result_sent`
45+
// is the tool-result-not-delivered root cause (a tool_use whose result never
46+
// came back — including a still-outstanding tool in a parallel turn).
47+
tool_result_sent: boolean;
48+
// True when an approval/permission gate fired. Only ACP runtimes surface this
49+
// (via an `acp_approval_request` diagnostic); stream/CLI runtimes bypass gates.
50+
approval_requested: boolean;
4251
artifact_write_seen: boolean;
4352
live_artifact_seen: boolean;
4453
// True when this run transparently re-seeded after an upstream session resume
@@ -160,6 +169,22 @@ export function summarizeRunDiagnosticsForAnalytics(args: {
160169
let stdout = '';
161170
let userVisibleOutputSeen = false;
162171
let toolCallSeen = false;
172+
// `tool_result_sent` = EVERY committed tool_use received a matching tool_result.
173+
// Paired by id (`tool_use.id` <-> `tool_result.toolUseId`, the same pairing
174+
// summarizeRunTimingAnalytics uses), because a plain "any tool_result after a
175+
// tool_use" flag reports delivered for a parallel turn like tool_use(A),
176+
// tool_use(B), tool_result(A) where B is still outstanding.
177+
//
178+
// Degraded provider events carry NO id, symmetrically on both sides — see
179+
// `agent-protocol/pi-rpc/events.ts` and `copilot-stream.ts`, which both emit
180+
// `toolCallId ?? null` for tool_use.id AND tool_result.toolUseId. Those are
181+
// paired by count instead; skipping them would let an unpaired id-less tool
182+
// call fall through to "delivered" and mask exactly the stall we're attributing.
183+
const outstandingToolUseIds = new Set<string>();
184+
let idlessToolUses = 0;
185+
let idlessToolResults = 0;
186+
let sawAnyToolUse = false;
187+
let approvalRequested = false;
163188
let artifactWriteSeen = args.artifactWriteSeen === true;
164189
let liveArtifactSeen = args.liveArtifactSeen === true;
165190
let recordedCloseReason: RunCloseReason | null = null;
@@ -183,7 +208,19 @@ export function summarizeRunDiagnosticsForAnalytics(args: {
183208
const delta = typeof data.delta === 'string' ? data.delta : '';
184209
if (delta.length > 0) userVisibleOutputSeen = true;
185210
}
186-
if (data.type === 'tool_use') toolCallSeen = true;
211+
if (data.type === 'tool_use') {
212+
toolCallSeen = true;
213+
sawAnyToolUse = true;
214+
if (typeof data.id === 'string') outstandingToolUseIds.add(data.id);
215+
else idlessToolUses += 1;
216+
}
217+
if (data.type === 'tool_result') {
218+
if (typeof data.toolUseId === 'string') outstandingToolUseIds.delete(data.toolUseId);
219+
else idlessToolResults += 1;
220+
}
221+
if (data.type === 'diagnostic' && data.name === 'acp_approval_request') {
222+
approvalRequested = true;
223+
}
187224
if (event.event === 'diagnostic' && data.type === 'agent_resume_auto_reseed') {
188225
resumeAutoReseeded = true;
189226
}
@@ -254,6 +291,11 @@ export function summarizeRunDiagnosticsForAnalytics(args: {
254291
first_token_seen: args.firstTokenSeen === true,
255292
user_visible_output_seen: userVisibleOutputSeen,
256293
tool_call_seen: toolCallSeen,
294+
tool_result_sent:
295+
sawAnyToolUse &&
296+
outstandingToolUseIds.size === 0 &&
297+
idlessToolResults >= idlessToolUses,
298+
approval_requested: approvalRequested,
257299
artifact_write_seen: artifactWriteSeen,
258300
live_artifact_seen: liveArtifactSeen,
259301
resume_auto_reseeded: resumeAutoReseeded,

apps/daemon/src/runtimes/chat-run-lifecycle.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -236,3 +236,31 @@ export function bufferedAntigravityGeminiFirstTokenAt(
236236
}
237237
return null;
238238
}
239+
240+
/**
241+
* Writes the composed prompt as the final chunk on the child's stdin, closes
242+
* it, and reports whether that write was backpressured.
243+
*
244+
* `end(chunk)` cannot report this: it returns the stream rather than a boolean,
245+
* and `writableNeedDrain` is already back to false by the time it returns — even
246+
* for a chunk that `write(chunk)` would have rejected. Every runtime except
247+
* Claude (which streams JSON and keeps stdin open) takes this path, so reading
248+
* backpressure off `end()` left `stdin_backpressure` permanently false on
249+
* exactly the runs whose `stdin_write` stalls it exists to attribute. Issuing
250+
* the write and the close separately is what makes the signal real.
251+
*
252+
* Returns true when the chunk had to be buffered because the OS pipe was full,
253+
* i.e. the child was not draining stdin.
254+
*/
255+
export function writePromptAndEndStdin(
256+
stdin: {
257+
write: (chunk: string, encoding: BufferEncoding, cb: (err?: Error | null) => void) => boolean;
258+
end: () => void;
259+
},
260+
composed: string,
261+
onFlush: (err?: Error | null) => void,
262+
): boolean {
263+
const accepted = stdin.write(composed, 'utf8', onFlush);
264+
stdin.end();
265+
return accepted === false;
266+
}

apps/daemon/src/runtimes/runs.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,14 @@ export function createChatRunService({
109109
retryOriginFailure: null,
110110
retryOriginErrorCode: null,
111111
stdinOpen: false,
112+
// E-lite root-cause telemetry. `stdinBackpressure` records whether the
113+
// prompt write to the child's stdin was queued (pipe buffer full — a
114+
// corroborating signal for a `stdin_write`-phase stall). `lastAgentActivityAt`
115+
// is the clock the inactivity watchdog keys off, read at finish to derive
116+
// `last_progress_age_ms`. (`approval_requested` and `tool_result_sent` are
117+
// derived from run.events by summarizeRunDiagnosticsForAnalytics.)
118+
stdinBackpressure: false,
119+
lastAgentActivityAt: now,
112120
// Work-completeness signals (#1247 / #1060), folded from agent events by
113121
// captureRunWorkCompletenessSignals (server.ts). `lastTodoSnapshot` is the
114122
// most recent TodoWrite `todos` array; `truncatedMidTurn` records a

apps/daemon/src/server.ts

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,7 @@ import {
9494
validateCodexGeneratedImagesDir,
9595
} from './runtimes/chat-prompt-inputs.js';
9696
import {
97+
writePromptAndEndStdin,
9798
applyClaudeStreamJsonRunBookkeeping,
9899
assertValidRuntimeDefInactivityTimeoutMs,
99100
bufferedAntigravityGeminiFirstTokenAt,
@@ -6267,6 +6268,9 @@ export async function startServer({
62676268
artifactRegistered,
62686269
});
62696270
const noteAgentActivity = () => {
6271+
// E-lite: stamp the last-activity clock BEFORE the disabled-watchdog bail
6272+
// so `last_progress_age_ms` is recorded even when the watchdog is off.
6273+
run.lastAgentActivityAt = Date.now();
62706274
const delay = activeInactivityTimeoutMs();
62716275
if (delay <= 0) return;
62726276
clearInactivityWatchdog();
@@ -8080,7 +8084,11 @@ export async function startServer({
80808084
},
80818085
});
80828086
try {
8083-
child.stdin.write(`${userMessage}\n`, 'utf8', markStdinWriteEnd);
8087+
// E-lite: `write` returns false when the chunk was buffered because the
8088+
// OS pipe is full (the child isn't draining stdin) — the corroborating
8089+
// signal for a `stdin_write`-phase inactivity stall.
8090+
const accepted = child.stdin.write(`${userMessage}\n`, 'utf8', markStdinWriteEnd);
8091+
run.stdinBackpressure = accepted === false;
80848092
} catch (err) {
80858093
// Swallow EPIPE here for the same reason as the listener above —
80868094
// a fast-exiting child has already routed its failure through
@@ -8089,7 +8097,9 @@ export async function startServer({
80898097
}
80908098
run.stdinOpen = true;
80918099
} else {
8092-
child.stdin.end(composed, 'utf8', markStdinWriteEnd);
8100+
// Split write + close so the boolean backpressure signal survives —
8101+
// see writePromptAndEndStdin for why `end(chunk)` cannot report it.
8102+
run.stdinBackpressure = writePromptAndEndStdin(child.stdin, composed, markStdinWriteEnd);
80938103
}
80948104
}
80958105
};
Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
import { describe, expect, it } from 'vitest';
2+
3+
import { writePromptAndEndStdin } from '../src/runtimes/chat-run-lifecycle.js';
4+
5+
// Regression guard for `run_finished.stdin_backpressure`.
6+
//
7+
// The first cut wrote the prompt with `child.stdin.end(composed, ...)` and then
8+
// read `child.stdin.writableNeedDrain`. That reports nothing: `end(chunk)`
9+
// returns the stream rather than a boolean, and `writableNeedDrain` is already
10+
// back to false by the time it returns — even for a chunk `write(chunk)` would
11+
// have rejected. Every runtime except Claude takes this path, so the field was
12+
// permanently false on exactly the runs whose `stdin_write` stalls it exists to
13+
// attribute.
14+
describe('writePromptAndEndStdin', () => {
15+
function fakeStdin(writeReturns: boolean) {
16+
const calls: { chunk: string; encoding: string }[] = [];
17+
const state = { ended: false, flushCb: null as null | ((err?: Error | null) => void) };
18+
return {
19+
calls,
20+
state,
21+
write(chunk: string, encoding: BufferEncoding, cb: (err?: Error | null) => void) {
22+
calls.push({ chunk, encoding });
23+
state.flushCb = cb;
24+
return writeReturns;
25+
},
26+
end() {
27+
state.ended = true;
28+
},
29+
};
30+
}
31+
32+
it('reports backpressure when the pipe rejected the chunk, and still closes stdin', () => {
33+
const stdin = fakeStdin(false);
34+
expect(writePromptAndEndStdin(stdin, 'prompt body', () => {})).toBe(true);
35+
expect(stdin.state.ended).toBe(true);
36+
expect(stdin.calls).toEqual([{ chunk: 'prompt body', encoding: 'utf8' }]);
37+
});
38+
39+
it('reports no backpressure when the chunk was accepted', () => {
40+
const stdin = fakeStdin(true);
41+
expect(writePromptAndEndStdin(stdin, 'prompt body', () => {})).toBe(false);
42+
expect(stdin.state.ended).toBe(true);
43+
});
44+
45+
it('forwards the flush callback so the stdin_write lifecycle mark still fires', () => {
46+
const stdin = fakeStdin(true);
47+
let flushed = 0;
48+
writePromptAndEndStdin(stdin, 'prompt body', () => {
49+
flushed += 1;
50+
});
51+
stdin.state.flushCb?.();
52+
expect(flushed).toBe(1);
53+
});
54+
});

apps/daemon/tests/run-diagnostics.test.ts

Lines changed: 101 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,8 @@ describe('run diagnostics', () => {
7676
first_token_seen: false,
7777
user_visible_output_seen: false,
7878
tool_call_seen: false,
79+
tool_result_sent: false,
80+
approval_requested: false,
7981
artifact_write_seen: false,
8082
live_artifact_seen: false,
8183
resume_auto_reseeded: false,
@@ -105,6 +107,8 @@ describe('run diagnostics', () => {
105107
first_token_seen: false,
106108
user_visible_output_seen: false,
107109
tool_call_seen: false,
110+
tool_result_sent: false,
111+
approval_requested: false,
108112
artifact_write_seen: false,
109113
live_artifact_seen: false,
110114
resume_auto_reseeded: false,
@@ -131,6 +135,8 @@ describe('run diagnostics', () => {
131135
first_token_seen: false,
132136
user_visible_output_seen: false,
133137
tool_call_seen: false,
138+
tool_result_sent: false,
139+
approval_requested: false,
134140
artifact_write_seen: false,
135141
live_artifact_seen: false,
136142
resume_auto_reseeded: false,
@@ -153,7 +159,7 @@ describe('run diagnostics', () => {
153159
const result = summarizeRunDiagnosticsForAnalytics({
154160
events: [
155161
{ event: 'stdout', data: { chunk: 'hello\n' } },
156-
{ event: 'agent', data: { type: 'tool_use', name: 'Read' } },
162+
{ event: 'agent', data: { type: 'tool_use', name: 'Read', id: 'tool-1' } },
157163
{ event: 'agent', data: { type: 'artifact' } },
158164
],
159165
exitCode: null,
@@ -170,8 +176,102 @@ describe('run diagnostics', () => {
170176
first_token_seen: true,
171177
user_visible_output_seen: true,
172178
tool_call_seen: true,
179+
// A tool_use with no following tool_result → not delivered.
180+
tool_result_sent: false,
181+
approval_requested: false,
173182
artifact_write_seen: true,
174183
live_artifact_seen: true,
175184
});
176185
});
186+
187+
it('flags tool_result_sent / approval_requested (E-lite root-cause discriminators)', () => {
188+
// Every committed tool_use resolved (paired by id) → delivered.
189+
const resolved = summarizeRunDiagnosticsForAnalytics({
190+
events: [
191+
{ event: 'agent', data: { type: 'tool_use', name: 'Read', id: 't1' } },
192+
{ event: 'agent', data: { type: 'tool_result', toolUseId: 't1' } },
193+
],
194+
exitCode: 0,
195+
signal: null,
196+
});
197+
expect(resolved.tool_call_seen).toBe(true);
198+
expect(resolved.tool_result_sent).toBe(true);
199+
200+
// Two parallel tool_uses both resolve (interleaved) → delivered.
201+
const parallelResolved = summarizeRunDiagnosticsForAnalytics({
202+
events: [
203+
{ event: 'agent', data: { type: 'tool_use', name: 'Read', id: 'a' } },
204+
{ event: 'agent', data: { type: 'tool_use', name: 'Grep', id: 'b' } },
205+
{ event: 'agent', data: { type: 'tool_result', toolUseId: 'a' } },
206+
{ event: 'agent', data: { type: 'tool_result', toolUseId: 'b' } },
207+
],
208+
exitCode: 0,
209+
signal: null,
210+
});
211+
expect(parallelResolved.tool_result_sent).toBe(true);
212+
213+
// The reviewer's regression case: tool_use(A), tool_use(B), tool_result(A).
214+
// B is still outstanding, so a result arriving for A must NOT mark delivered.
215+
const parallelHung = summarizeRunDiagnosticsForAnalytics({
216+
events: [
217+
{ event: 'agent', data: { type: 'tool_use', name: 'Read', id: 'a' } },
218+
{ event: 'agent', data: { type: 'tool_use', name: 'Bash', id: 'b' } },
219+
{ event: 'agent', data: { type: 'tool_result', toolUseId: 'a' } },
220+
],
221+
exitCode: null,
222+
signal: 'SIGKILL',
223+
});
224+
expect(parallelHung.tool_call_seen).toBe(true);
225+
expect(parallelHung.tool_result_sent).toBe(false);
226+
227+
// The last tool_use of a sequential turn has no result → not delivered.
228+
const hung = summarizeRunDiagnosticsForAnalytics({
229+
events: [
230+
{ event: 'agent', data: { type: 'tool_use', name: 'Read', id: 'a' } },
231+
{ event: 'agent', data: { type: 'tool_result', toolUseId: 'a' } },
232+
{ event: 'agent', data: { type: 'tool_use', name: 'Bash', id: 'b' } },
233+
],
234+
exitCode: null,
235+
signal: 'SIGKILL',
236+
});
237+
expect(hung.tool_call_seen).toBe(true);
238+
expect(hung.tool_result_sent).toBe(false);
239+
240+
// Degraded provider events carry a null id on BOTH sides (pi-rpc,
241+
// copilot-stream emit `toolCallId ?? null`). An unpaired id-less tool call
242+
// must NOT fall through to "delivered".
243+
const idlessHung = summarizeRunDiagnosticsForAnalytics({
244+
events: [
245+
{ event: 'agent', data: { type: 'tool_use', name: 'Read', id: null } },
246+
],
247+
exitCode: null,
248+
signal: 'SIGKILL',
249+
});
250+
expect(idlessHung.tool_call_seen).toBe(true);
251+
expect(idlessHung.tool_result_sent).toBe(false);
252+
253+
// ...but an id-less tool call that DID get its (also id-less) result counts.
254+
const idlessResolved = summarizeRunDiagnosticsForAnalytics({
255+
events: [
256+
{ event: 'agent', data: { type: 'tool_use', name: 'Read', id: null } },
257+
{ event: 'agent', data: { type: 'tool_result', toolUseId: null } },
258+
],
259+
exitCode: 0,
260+
signal: null,
261+
});
262+
expect(idlessResolved.tool_result_sent).toBe(true);
263+
264+
// An ACP approval diagnostic flips approval_requested.
265+
const approved = summarizeRunDiagnosticsForAnalytics({
266+
events: [
267+
{
268+
event: 'agent',
269+
data: { type: 'diagnostic', name: 'acp_approval_request', optionId: 'allow_once' },
270+
},
271+
],
272+
exitCode: 0,
273+
signal: null,
274+
});
275+
expect(approved.approval_requested).toBe(true);
276+
});
177277
});

0 commit comments

Comments
 (0)