Skip to content

Commit 9476dba

Browse files
authored
fix(agent): drop console export and resolve root span status from latest turn
Addresses two external review findings: - _posthog/console records are no longer exported to PostHog Logs. Console lines are free-text agent-server diagnostics that interpolate arbitrary data - the user-message handler logs a 100-char prompt preview and the extension-notification handler stringifies params into that channel - so exporting them verbatim violated the metadata-only allowlist. They remain in the S3 session log and the event-ingest stream. Regression tests assert console entries (including an interpolated prompt preview) never map to an exported record. - The task_run root span status is now resolved at shutdown from the LATEST turn outcome instead of being set sticky-OK on the first clean end_turn. A multi-turn run whose final turn is cancelled/refused no longer reports OK; a run error still always wins. The root span only exports at end, so per-turn status writes were cosmetic anyway. Stacked on posthog-code/agent-run-otel-telemetry (pure fast-forward). Generated-By: PostHog Code Task-Id: 0c511836-2180-455a-9b58-45df2a0661ec
1 parent 29e014f commit 9476dba

3 files changed

Lines changed: 74 additions & 50 deletions

File tree

packages/agent/src/otel-telemetry.test.ts

Lines changed: 37 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -170,21 +170,6 @@ describe("OtelRunTelemetry", () => {
170170
body: "error: boom",
171171
attrs: { error_source: "agent_server" },
172172
},
173-
{
174-
name: "console warn",
175-
entry: makeEntry("_posthog/console", {
176-
level: "warn",
177-
message: "careful",
178-
}),
179-
severityText: "WARN",
180-
body: "careful",
181-
},
182-
{
183-
name: "console with unknown level",
184-
entry: makeEntry("_posthog/console", { level: "silly", message: "m" }),
185-
severityText: "INFO",
186-
body: "m",
187-
},
188173
{
189174
name: "progress",
190175
entry: makeEntry("_posthog/progress", {
@@ -321,16 +306,29 @@ describe("OtelRunTelemetry", () => {
321306
"user_message",
322307
makeEntry("_posthog/user_message", { message: "SECRET" }),
323308
],
309+
// Console lines are free-text agent-server diagnostics; some interpolate
310+
// content (e.g. the prompt preview logged on user_message handling).
311+
[
312+
"console with interpolated prompt preview",
313+
makeEntry("_posthog/console", {
314+
level: "debug",
315+
message: "Processing user message (detectedPrUrl=none): SECRET...",
316+
}),
317+
],
318+
[
319+
"console error",
320+
makeEntry("_posthog/console", { level: "error", message: "SECRET" }),
321+
],
324322
["unknown extension method", makeEntry("_posthog/some_new_event", {})],
325323
])("drops %s", (_name, entry) => {
326324
expect(mapNotificationToLogRecord(entry)).toBeNull();
327325
});
328326

329327
it("caps body length", () => {
330328
const mapped = mapNotificationToLogRecord(
331-
makeEntry("_posthog/console", {
332-
level: "info",
333-
message: "x".repeat(5000),
329+
makeEntry("_posthog/error", {
330+
source: "agent_server",
331+
error: "x".repeat(5000),
334332
}),
335333
);
336334

@@ -531,6 +529,27 @@ describe("OtelRunTelemetry", () => {
531529
expect(surface).not.toContain(".env");
532530
});
533531

532+
it("does not leave root OK when a later turn ends non-clean", async () => {
533+
driveSuccessfulRun();
534+
// Second turn gets cancelled: the latest outcome wins, so the earlier
535+
// clean turn must not leave the run marked OK.
536+
telemetry.append(
537+
RUN_ID,
538+
makeEntry("session/prompt", {
539+
prompt: [{ type: "text", text: "again" }],
540+
}),
541+
);
542+
telemetry.append(
543+
RUN_ID,
544+
makeEntry("_posthog/turn_complete", { stopReason: "cancelled" }),
545+
);
546+
547+
await telemetry.shutdown();
548+
549+
expect(spanByName("task_run").status.code).toBe(SpanStatusCode.UNSET);
550+
expect(exportedSpans().filter((s) => s.name === "turn")).toHaveLength(2);
551+
});
552+
534553
it("stamps log records with the span they belong to", async () => {
535554
driveSuccessfulRun();
536555

packages/agent/src/otel-telemetry.ts

Lines changed: 8 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -67,13 +67,6 @@ export interface MappedLogRecord {
6767
attributes: Attributes;
6868
}
6969

70-
const CONSOLE_SEVERITIES: Record<string, [SeverityNumber, string]> = {
71-
debug: [SeverityNumber.DEBUG, "DEBUG"],
72-
info: [SeverityNumber.INFO, "INFO"],
73-
warn: [SeverityNumber.WARN, "WARN"],
74-
error: [SeverityNumber.ERROR, "ERROR"],
75-
};
76-
7770
function record(
7871
severity: [SeverityNumber, string],
7972
body: string,
@@ -88,9 +81,9 @@ function record(
8881
};
8982
}
9083

91-
const INFO = CONSOLE_SEVERITIES.info;
92-
const WARN = CONSOLE_SEVERITIES.warn;
93-
const ERROR = CONSOLE_SEVERITIES.error;
84+
const INFO: [SeverityNumber, string] = [SeverityNumber.INFO, "INFO"];
85+
const WARN: [SeverityNumber, string] = [SeverityNumber.WARN, "WARN"];
86+
const ERROR: [SeverityNumber, string] = [SeverityNumber.ERROR, "ERROR"];
9487

9588
function mapSessionUpdate(
9689
method: string,
@@ -206,15 +199,11 @@ export function mapNotificationToLogRecord(
206199
typeof params.error === "string" ? params.error : "unknown error";
207200
return record(ERROR, `error: ${message}`, method, attrs);
208201
}
209-
case POSTHOG_NOTIFICATIONS.CONSOLE: {
210-
const severity =
211-
(typeof params.level === "string" &&
212-
CONSOLE_SEVERITIES[params.level]) ||
213-
INFO;
214-
const message =
215-
typeof params.message === "string" ? params.message : "console output";
216-
return record(severity, message, method, {});
217-
}
202+
// POSTHOG_NOTIFICATIONS.CONSOLE is deliberately NOT exported: those are
203+
// free-text agent-server diagnostics that interpolate arbitrary data
204+
// (prompt previews, stringified extension params), so shipping them would
205+
// leak content the allowlist exists to keep in the sandbox. They remain
206+
// in the S3 session log and the event-ingest stream.
218207
case POSTHOG_NOTIFICATIONS.PROGRESS: {
219208
const attrs: Attributes = {};
220209
strAttr(attrs, "progress_group", params.group);

packages/agent/src/otel-trace-builder.ts

Lines changed: 29 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,12 @@ export interface RunTraceBuilderConfig {
4141
* the corresponding log record should be emitted under, so logs and spans
4242
* cross-link in the UI via trace_id/span_id.
4343
*
44-
* Root span status: OK once a turn ends cleanly (`end_turn`) or on
45-
* task_complete, ERROR on a run error (which always wins), unset when the run
46-
* ends without either (cancelled / timed out).
44+
* Root span status is resolved at shutdown from the LATEST turn outcome (the
45+
* root span only exports when it ends, so earlier turns must not leave a
46+
* sticky OK): OK when the last turn ended cleanly (`end_turn`, or an explicit
47+
* task_complete), ERROR on a run error (which always wins) or a last turn
48+
* that stopped with `error`, unset otherwise (cancelled / refused / timed out
49+
* / no completed turns).
4750
*
4851
* Spans carry the same allowlist stance as the log export: lifecycle, status,
4952
* usage, and identifiers only — never prompts, tool arguments, or output.
@@ -54,6 +57,8 @@ export class RunTraceBuilder {
5457
private rootSpan: Span;
5558
private rootContext: Context;
5659
private rootErrored = false;
60+
/** stopReason of the most recent completed turn; drives root status at shutdown */
61+
private lastStopReason?: string;
5762
private turnSpan?: Span;
5863
private turnContext?: Context;
5964
private turnIndex = 0;
@@ -109,9 +114,9 @@ export class RunTraceBuilder {
109114
return this.currentContext();
110115
}
111116
case POSTHOG_NOTIFICATIONS.TASK_COMPLETE:
112-
if (!this.rootErrored) {
113-
this.rootSpan.setStatus({ code: SpanStatusCode.OK });
114-
}
117+
// Explicit success signal (forward-compat; production decides the
118+
// terminal status outside the sandbox) — treated as a clean outcome.
119+
this.lastStopReason = "end_turn";
115120
return this.rootContext;
116121
case POSTHOG_NOTIFICATIONS.ERROR:
117122
return this.handleError(params, time);
@@ -126,13 +131,25 @@ export class RunTraceBuilder {
126131
await this.provider.forceFlush();
127132
}
128133

129-
/** Ends any open spans (status unset), then flushes and stops the provider. */
134+
/**
135+
* Ends any open spans (status unset), resolves the root status from the
136+
* latest turn outcome, then flushes and stops the provider.
137+
*/
130138
async shutdown(): Promise<void> {
131139
if (this.ended) return;
132140
this.ended = true;
133141
const now = new Date();
134142
this.closeOpenTools(now);
135143
this.closeTurn(undefined, now);
144+
if (!this.rootErrored) {
145+
if (this.lastStopReason === "end_turn") {
146+
this.rootSpan.setStatus({ code: SpanStatusCode.OK });
147+
} else if (this.lastStopReason === "error") {
148+
this.rootSpan.setStatus({ code: SpanStatusCode.ERROR });
149+
}
150+
// Any other latest outcome (cancelled, refusal, max_tokens, none)
151+
// leaves the status unset: neither success nor failure.
152+
}
136153
this.rootSpan.end(now);
137154
await this.provider.shutdown();
138155
}
@@ -167,12 +184,11 @@ export class RunTraceBuilder {
167184
this.closeOpenTools(time);
168185
this.closeTurn({ stopReason, errored: stopReason === "error" }, time);
169186
// The sandbox never emits task_complete for successful runs (the terminal
170-
// "completed" status is decided by the workflow outside), so a cleanly
171-
// finished turn is the success signal for the run. A later error still
172-
// wins via rootErrored/handleError.
173-
if (stopReason === "end_turn" && !this.rootErrored) {
174-
this.rootSpan.setStatus({ code: SpanStatusCode.OK });
175-
}
187+
// "completed" status is decided by the workflow outside), so the latest
188+
// turn outcome is the run's success signal — recorded here, resolved into
189+
// the root span status at shutdown so an early clean turn can't leave a
190+
// stale OK on a run whose last turn was cancelled.
191+
this.lastStopReason = stopReason;
176192
return context;
177193
}
178194

0 commit comments

Comments
 (0)