From 7b0c2be5de98cd0366a4c1c2d9f69c6d21667398 Mon Sep 17 00:00:00 2001 From: Alessandro Pogliaghi Date: Mon, 6 Jul 2026 17:17:13 +0100 Subject: [PATCH 01/11] feat(agent): export cloud run telemetry to PostHog Logs and APM over OTLP Adds OtelRunTelemetry, a SessionLogWriter sink that ships an allowlisted metadata subset of the session log (run/turn/tool lifecycle, usage, errors; never message content or tool arguments) to PostHog Logs, and, when a traces URL is configured, builds one APM trace per run (task_run root span, a turn span per prompt, a tool_call: span per tool call) with trace/span ids stamped on log records so Logs and APM cross-link. Resource attributes carry run_id/task_id/team_id/user_id/distinct_id so cloud runs are filterable per user in the Logs UI. Configured via POSTHOG_AGENT_OTEL_LOGS_URL/_TOKEN (+ optional POSTHOG_AGENT_OTEL_TRACES_URL), deliberately not standard OTEL_* names so OTel SDKs in user code running in the sandbox never auto-export into the telemetry project. Telemetry stays off unless the logs pair is set, flushes on session cleanup/terminal errors, and can never break session log persistence (sink failures are isolated). Replaces the unwired otel-log-writer whose default endpoint (/i/v1/agent-logs) does not exist in the ingest service, and removes the dead AgentConfig.otelTransport field. Generated-By: PostHog Code Task-Id: 0c511836-2180-455a-9b58-45df2a0661ec --- packages/agent/README.md | 8 + packages/agent/package.json | 9 +- packages/agent/src/otel-attributes.ts | 61 ++ packages/agent/src/otel-log-writer.test.ts | 105 --- packages/agent/src/otel-log-writer.ts | 94 --- packages/agent/src/otel-telemetry.test.ts | 613 ++++++++++++++++++ packages/agent/src/otel-telemetry.ts | 365 +++++++++++ packages/agent/src/otel-trace-builder.ts | 274 ++++++++ packages/agent/src/server/agent-server.ts | 77 ++- packages/agent/src/server/bin.ts | 8 + packages/agent/src/server/types.ts | 6 + packages/agent/src/session-log-writer.test.ts | 46 ++ packages/agent/src/session-log-writer.ts | 50 +- packages/agent/src/types.ts | 11 - pnpm-lock.yaml | 126 +++- 15 files changed, 1604 insertions(+), 249 deletions(-) create mode 100644 packages/agent/src/otel-attributes.ts delete mode 100644 packages/agent/src/otel-log-writer.test.ts delete mode 100644 packages/agent/src/otel-log-writer.ts create mode 100644 packages/agent/src/otel-telemetry.test.ts create mode 100644 packages/agent/src/otel-telemetry.ts create mode 100644 packages/agent/src/otel-trace-builder.ts diff --git a/packages/agent/README.md b/packages/agent/README.md index 29d54f9693..71247db686 100644 --- a/packages/agent/README.md +++ b/packages/agent/README.md @@ -166,6 +166,14 @@ Required environment variables (validated by zod in `src/server/bin.ts`): - `POSTHOG_PERSONAL_API_KEY` — API key for PostHog requests - `POSTHOG_PROJECT_ID` — numeric project ID +Optional run telemetry (the logs pair must both be set, otherwise telemetry stays off): + +- `POSTHOG_AGENT_OTEL_LOGS_URL` — full OTLP logs URL for run metadata, e.g. `https://us.i.posthog.com/i/v1/logs` +- `POSTHOG_AGENT_OTEL_LOGS_TOKEN` — project API key of the telemetry project +- `POSTHOG_AGENT_OTEL_TRACES_URL` — full OTLP traces URL, e.g. `https://us.i.posthog.com/i/v1/traces`; additionally enables one APM trace per run + +When set, `AgentServer` ships an allowlisted metadata subset of the session log (run/turn/tool lifecycle, usage, errors — never message content or tool arguments; see `src/otel-telemetry.ts`) to PostHog Logs, tagged with `service.name=posthog-code-agent` and `run_id`/`task_id`/`team_id`/`user_id`/`distinct_id` resource attributes so cloud runs are filterable per user. With the traces URL set, each run also produces an APM trace (`task_run` root span, a `turn` span per prompt, a `tool_call:` span per tool call; see `src/otel-trace-builder.ts`), and log records carry the matching trace/span ids so Logs and APM cross-link. + ## Agent SDK The `Agent` class (`src/agent.ts`) is the entrypoint for local/programmatic usage. It handles LLM gateway configuration, log writer setup, and model filtering — then delegates to `createAcpConnection()`. diff --git a/packages/agent/package.json b/packages/agent/package.json index 31004ac74a..b1b2157926 100644 --- a/packages/agent/package.json +++ b/packages/agent/package.json @@ -117,9 +117,9 @@ "node": ">=20.0.0" }, "devDependencies": { - "@posthog/shared": "workspace:*", - "@posthog/git": "workspace:*", "@posthog/enricher": "workspace:*", + "@posthog/git": "workspace:*", + "@posthog/shared": "workspace:*", "@types/bun": "latest", "@types/tar": "^6.1.13", "msw": "^2.12.7", @@ -133,11 +133,15 @@ "@anthropic-ai/claude-agent-sdk": "0.3.197", "@anthropic-ai/sdk": "0.109.0", "@hono/node-server": "^1.19.9", + "@modelcontextprotocol/sdk": "1.29.0", "@openai/codex": "0.140.0", + "@opentelemetry/api": "^1.9.1", "@opentelemetry/api-logs": "^0.208.0", "@opentelemetry/exporter-logs-otlp-http": "^0.208.0", + "@opentelemetry/exporter-trace-otlp-http": "^0.208.0", "@opentelemetry/resources": "^2.0.0", "@opentelemetry/sdk-logs": "^0.208.0", + "@opentelemetry/sdk-trace-base": "^2.8.0", "@opentelemetry/semantic-conventions": "^1.28.0", "@types/jsonwebtoken": "^9.0.10", "commander": "^14.0.2", @@ -145,7 +149,6 @@ "hono": "^4.11.7", "jsonwebtoken": "^9.0.2", "minimatch": "^10.0.3", - "@modelcontextprotocol/sdk": "1.29.0", "tar": "^7.5.0", "uuid": "13.0.0", "yoga-wasm-web": "^0.3.3", diff --git a/packages/agent/src/otel-attributes.ts b/packages/agent/src/otel-attributes.ts new file mode 100644 index 0000000000..b2014c5d06 --- /dev/null +++ b/packages/agent/src/otel-attributes.ts @@ -0,0 +1,61 @@ +const MAX_BODY_CHARS = 2000; +// PostHog Logs only facets attribute key/value pairs shorter than 256 chars, +// so free-text attribute values are capped well below that. +const MAX_ATTR_CHARS = 200; + +export type AttributeValue = string | number | boolean; +export type Attributes = Record; + +export { MAX_ATTR_CHARS, MAX_BODY_CHARS }; + +export function truncate(value: string, max: number): string { + return value.length <= max ? value : `${value.slice(0, max)}…`; +} + +export function asRecord(value: unknown): Record | undefined { + return typeof value === "object" && value !== null && !Array.isArray(value) + ? (value as Record) + : undefined; +} + +export function strAttr( + attrs: Attributes, + key: string, + value: unknown, + max = MAX_ATTR_CHARS, +): string | undefined { + if (typeof value !== "string" || value.length === 0) return undefined; + const truncated = truncate(value, max); + attrs[key] = truncated; + return truncated; +} + +export function numAttr(attrs: Attributes, key: string, value: unknown): void { + if (typeof value === "number" && Number.isFinite(value)) { + attrs[key] = value; + } +} + +export function usageAttributes(params: Record): Attributes { + const attrs: Attributes = {}; + const used = asRecord(params.used); + if (used) { + numAttr(attrs, "tokens_input", used.inputTokens); + numAttr(attrs, "tokens_output", used.outputTokens); + numAttr(attrs, "tokens_cached_read", used.cachedReadTokens); + numAttr(attrs, "tokens_cached_write", used.cachedWriteTokens); + } + // Claude sends a plain number; other shapes carry { amount }. + const cost = + typeof params.cost === "number" + ? params.cost + : asRecord(params.cost)?.amount; + numAttr(attrs, "cost_usd", cost); + return attrs; +} + +/** Timestamp of a stored entry as a Date, falling back to now when invalid. */ +export function entryTime(timestamp: string): Date { + const parsed = new Date(timestamp); + return Number.isNaN(parsed.getTime()) ? new Date() : parsed; +} diff --git a/packages/agent/src/otel-log-writer.test.ts b/packages/agent/src/otel-log-writer.test.ts deleted file mode 100644 index 29fc78f035..0000000000 --- a/packages/agent/src/otel-log-writer.test.ts +++ /dev/null @@ -1,105 +0,0 @@ -import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; -import { OtelLogWriter } from "./otel-log-writer"; -import type { StoredNotification } from "./types"; - -// Mock the OTEL exporter -const mockExport = vi.fn((_logs, callback) => { - callback({ code: 0 }); // Success -}); - -vi.mock("@opentelemetry/exporter-logs-otlp-http", () => ({ - OTLPLogExporter: class { - export = mockExport; - shutdown = vi.fn().mockResolvedValue(undefined); - }, -})); - -describe("OtelLogWriter", () => { - let writer: OtelLogWriter; - - beforeEach(() => { - mockExport.mockClear(); - // Session context (taskId, runId) is now passed in constructor as resource attributes - writer = new OtelLogWriter( - { - posthogHost: "https://us.i.posthog.com", - apiKey: "phc_test_key", - flushIntervalMs: 100, - }, - { - taskId: "task-123", - runId: "run-456", - }, - ); - }); - - afterEach(async () => { - await writer.shutdown(); - }); - - it("should emit a log entry with event_type as regular attribute", async () => { - const notification: StoredNotification = { - type: "notification", - timestamp: new Date().toISOString(), - notification: { - jsonrpc: "2.0", - method: "_posthog/test_event", - params: { foo: "bar" }, - }, - }; - - // taskId and runId are now resource attributes set in constructor, - // only notification is passed per-emit - writer.emit({ notification }); - - // Force flush to trigger export - await writer.flush(); - - // Verify export was called - expect(mockExport).toHaveBeenCalled(); - - // Get the logs that were exported - const exportedLogs = mockExport.mock.calls[0][0]; - expect(exportedLogs.length).toBe(1); - - const log = exportedLogs[0]; - // task_id and run_id are now resource attributes, not regular attributes - expect(log.attributes.task_id).toBeUndefined(); - expect(log.attributes.run_id).toBeUndefined(); - // event_type is still a regular attribute (varies per log entry) - expect(log.attributes.event_type).toBe("_posthog/test_event"); - expect(log.body).toBe(JSON.stringify(notification)); - - // Verify resource attributes contain task_id and run_id - expect(log.resource.attributes.task_id).toBe("task-123"); - expect(log.resource.attributes.run_id).toBe("run-456"); - expect(log.resource.attributes["service.name"]).toBe("posthog-code-agent"); - }); - - it("should batch multiple log entries", async () => { - const makeNotification = (method: string): StoredNotification => ({ - type: "notification", - timestamp: new Date().toISOString(), - notification: { - jsonrpc: "2.0", - method, - }, - }); - - writer.emit({ notification: makeNotification("event_1") }); - writer.emit({ notification: makeNotification("event_2") }); - writer.emit({ notification: makeNotification("event_3") }); - - await writer.flush(); - - expect(mockExport).toHaveBeenCalled(); - const exportedLogs = mockExport.mock.calls[0][0]; - expect(exportedLogs.length).toBe(3); - - // All logs should share the same resource attributes - for (const log of exportedLogs) { - expect(log.resource.attributes.task_id).toBe("task-123"); - expect(log.resource.attributes.run_id).toBe("run-456"); - } - }); -}); diff --git a/packages/agent/src/otel-log-writer.ts b/packages/agent/src/otel-log-writer.ts deleted file mode 100644 index 83099749ae..0000000000 --- a/packages/agent/src/otel-log-writer.ts +++ /dev/null @@ -1,94 +0,0 @@ -import { SeverityNumber } from "@opentelemetry/api-logs"; -import { OTLPLogExporter } from "@opentelemetry/exporter-logs-otlp-http"; -import { resourceFromAttributes } from "@opentelemetry/resources"; -import { - BatchLogRecordProcessor, - LoggerProvider, -} from "@opentelemetry/sdk-logs"; -import { ATTR_SERVICE_NAME } from "@opentelemetry/semantic-conventions"; -import type { StoredNotification } from "./types"; -import type { Logger } from "./utils/logger"; - -export interface OtelLogConfig { - /** PostHog ingest host, e.g., "https://us.i.posthog.com" */ - posthogHost: string; - /** Project API key, e.g., "phc_xxx" */ - apiKey: string; - /** Batch flush interval in ms (default: 500) */ - flushIntervalMs?: number; - /** Override the logs endpoint path (default: /i/v1/agent-logs) */ - logsPath?: string; -} - -/** - * Session context for resource attributes. - * These are set once per OTEL logger instance and indexed via resource_fingerprint - */ -export interface SessionContext { - /** Parent task grouping - all runs for a task share this */ - taskId: string; - /** Primary conversation identifier - all events in a run share this */ - runId: string; - /** Deployment environment - "local" for desktop, "cloud" for cloud sandbox */ - deviceType?: "local" | "cloud"; -} - -export class OtelLogWriter { - private loggerProvider: LoggerProvider; - private logger: ReturnType; - - constructor( - config: OtelLogConfig, - sessionContext: SessionContext, - _debugLogger?: Logger, - ) { - const logsPath = config.logsPath ?? "/i/v1/agent-logs"; - const exporter = new OTLPLogExporter({ - url: `${config.posthogHost}${logsPath}`, - headers: { Authorization: `Bearer ${config.apiKey}` }, - }); - - const processor = new BatchLogRecordProcessor(exporter, { - scheduledDelayMillis: config.flushIntervalMs ?? 500, - }); - - // Resource attributes are set ONCE per session and indexed via resource_fingerprint - // So we have fast queries by run_id/task_id in PostHog Logs UI - this.loggerProvider = new LoggerProvider({ - resource: resourceFromAttributes({ - [ATTR_SERVICE_NAME]: "posthog-code-agent", - run_id: sessionContext.runId, - task_id: sessionContext.taskId, - device_type: sessionContext.deviceType ?? "local", - }), - processors: [processor], - }); - - this.logger = this.loggerProvider.getLogger("agent-session"); - } - - /** - * Emit an agent event to PostHog Logs via OTEL. - */ - emit(entry: { notification: StoredNotification }): void { - const { notification } = entry; - const eventType = notification.notification.method; - - this.logger.emit({ - severityNumber: SeverityNumber.INFO, - severityText: "INFO", - body: JSON.stringify(notification), - attributes: { - event_type: eventType, - }, - }); - } - - async flush(): Promise { - await this.loggerProvider.forceFlush(); - } - - async shutdown(): Promise { - await this.loggerProvider.shutdown(); - } -} diff --git a/packages/agent/src/otel-telemetry.test.ts b/packages/agent/src/otel-telemetry.test.ts new file mode 100644 index 0000000000..eba2c86a54 --- /dev/null +++ b/packages/agent/src/otel-telemetry.test.ts @@ -0,0 +1,613 @@ +import { SpanKind, SpanStatusCode } from "@opentelemetry/api"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { mapNotificationToLogRecord, OtelRunTelemetry } from "./otel-telemetry"; +import type { StoredNotification } from "./types"; + +const mockLogExport = vi.fn((_logs, callback) => { + callback({ code: 0 }); // Success +}); + +const mockSpanExport = vi.fn((_spans, callback) => { + callback({ code: 0 }); // Success +}); + +vi.mock("@opentelemetry/exporter-logs-otlp-http", () => ({ + OTLPLogExporter: class { + export = mockLogExport; + shutdown = vi.fn().mockResolvedValue(undefined); + }, +})); + +vi.mock("@opentelemetry/exporter-trace-otlp-http", () => ({ + OTLPTraceExporter: class { + export = mockSpanExport; + shutdown = vi.fn().mockResolvedValue(undefined); + }, +})); + +const RUN_ID = "run-456"; + +const RESOURCE = { + taskId: "task-123", + runId: RUN_ID, + deviceType: "cloud" as const, + teamId: 42, + userId: 7, + distinctId: "distinct-1", + adapter: "claude", + mode: "background", + agentVersion: "1.2.3", +}; + +function makeEntry( + method: string, + params?: Record, +): StoredNotification { + return { + type: "notification", + timestamp: "2026-07-06T12:00:00.000Z", + notification: { jsonrpc: "2.0", method, params }, + }; +} + +function sessionUpdate(update: Record): StoredNotification { + return makeEntry("session/update", { update }); +} + +interface ExportedLog { + body: string; + attributes: Record; + resource: { attributes: Record }; + spanContext?: { traceId: string; spanId: string }; +} + +interface ExportedSpan { + name: string; + kind: number; + status: { code: number; message?: string }; + attributes: Record; + parentSpanContext?: { spanId: string }; + spanContext: () => { traceId: string; spanId: string }; +} + +function exportedLogs(): ExportedLog[] { + return mockLogExport.mock.calls.flatMap((call) => call[0]); +} + +function exportedSpans(): ExportedSpan[] { + return mockSpanExport.mock.calls.flatMap((call) => call[0]); +} + +function spanByName(name: string): ExportedSpan { + const span = exportedSpans().find((s) => s.name === name); + expect(span, `span ${name} should be exported`).toBeDefined(); + return span as ExportedSpan; +} + +describe("OtelRunTelemetry", () => { + beforeEach(() => { + mockLogExport.mockClear(); + mockSpanExport.mockClear(); + }); + + describe("mapNotificationToLogRecord", () => { + it.each([ + { + name: "run_started", + entry: makeEntry("_posthog/run_started", { + agentVersion: "1.0.0", + sessionId: "acp-1", + }), + body: "run started", + attrs: { + event_type: "_posthog/run_started", + agent_version: "1.0.0", + session_id: "acp-1", + }, + }, + { + name: "double-prefixed extension method", + entry: makeEntry("__posthog/run_started", {}), + body: "run started", + attrs: { event_type: "_posthog/run_started" }, + }, + { + name: "sdk_session", + entry: makeEntry("_posthog/sdk_session", { + adapter: "claude", + sessionId: "acp-1", + }), + body: "sdk session created (claude)", + attrs: { adapter: "claude", session_id: "acp-1" }, + }, + { + name: "usage_update with numeric cost", + entry: makeEntry("_posthog/usage_update", { + used: { + inputTokens: 100, + outputTokens: 20, + cachedReadTokens: 5, + cachedWriteTokens: 2, + }, + cost: 0.42, + }), + body: "usage update", + attrs: { + tokens_input: 100, + tokens_output: 20, + tokens_cached_read: 5, + tokens_cached_write: 2, + cost_usd: 0.42, + }, + }, + { + name: "usage_update with cost object", + entry: makeEntry("_posthog/usage_update", { + cost: { amount: 1.5, currency: "USD" }, + }), + body: "usage update", + attrs: { cost_usd: 1.5 }, + }, + { + name: "turn_complete", + entry: makeEntry("_posthog/turn_complete", { stopReason: "end_turn" }), + body: "turn complete (end_turn)", + attrs: { stop_reason: "end_turn" }, + }, + { + name: "task_complete", + entry: makeEntry("_posthog/task_complete", {}), + body: "task complete", + attrs: { event_type: "_posthog/task_complete" }, + }, + { + name: "error", + entry: makeEntry("_posthog/error", { + source: "agent_server", + error: "boom", + }), + severityText: "ERROR", + body: "error: boom", + attrs: { error_source: "agent_server" }, + }, + { + name: "console warn", + entry: makeEntry("_posthog/console", { + level: "warn", + message: "careful", + }), + severityText: "WARN", + body: "careful", + }, + { + name: "console with unknown level", + entry: makeEntry("_posthog/console", { level: "silly", message: "m" }), + severityText: "INFO", + body: "m", + }, + { + name: "progress", + entry: makeEntry("_posthog/progress", { + group: "setup:run-1", + step: "agent", + status: "completed", + label: "Started agent", + }), + body: "progress: agent completed (Started agent)", + attrs: { + progress_group: "setup:run-1", + progress_step: "agent", + progress_status: "completed", + }, + }, + { + name: "git_checkpoint", + entry: makeEntry("_posthog/git_checkpoint", { + branch: "posthog-code/fix", + }), + body: "git checkpoint", + attrs: { branch: "posthog-code/fix" }, + }, + { + name: "branch_created", + entry: makeEntry("_posthog/branch_created", { branch: "b1" }), + body: "branch created", + attrs: { branch: "b1" }, + }, + { + name: "permission_request without tool content", + entry: makeEntry("_posthog/permission_request", { + requestId: "r1", + toolCallId: "t1", + toolCall: { title: "rm -rf /" }, + }), + body: "permission request", + attrs: { request_id: "r1", tool_call_id: "t1" }, + }, + { + name: "tool_call start", + entry: sessionUpdate({ + sessionUpdate: "tool_call", + toolCallId: "t1", + kind: "execute", + status: "pending", + title: "Run tests", + }), + body: "tool call started (execute)", + attrs: { + session_update_type: "tool_call", + tool_call_id: "t1", + tool_kind: "execute", + tool_status: "pending", + }, + }, + { + name: "terminal tool_call_update completed", + entry: sessionUpdate({ + sessionUpdate: "tool_call_update", + toolCallId: "t1", + status: "completed", + }), + body: "tool call completed", + attrs: { tool_call_id: "t1", tool_status: "completed" }, + }, + { + name: "terminal tool_call_update failed", + entry: sessionUpdate({ + sessionUpdate: "tool_call_update", + toolCallId: "t1", + status: "failed", + }), + severityText: "WARN", + body: "tool call failed", + }, + ])("maps $name", ({ entry, severityText = "INFO", body, attrs = {} }) => { + const mapped = mapNotificationToLogRecord(entry); + + expect(mapped).not.toBeNull(); + expect(mapped?.severityText).toBe(severityText); + expect(mapped?.body).toBe(body); + expect(mapped?.attributes).toMatchObject(attrs); + }); + + // Content-bearing notifications must stay in the session log: exporting + // them would ship customer prompts and repo content to the telemetry + // project, and in-progress tool snapshots would multiply billed bytes. + it.each([ + [ + "agent_message", + sessionUpdate({ + sessionUpdate: "agent_message", + content: { type: "text", text: "SECRET" }, + }), + ], + [ + "agent_message_chunk", + sessionUpdate({ + sessionUpdate: "agent_message_chunk", + content: { type: "text", text: "SECRET" }, + }), + ], + [ + "agent_thought_chunk", + sessionUpdate({ + sessionUpdate: "agent_thought_chunk", + content: { type: "text", text: "SECRET" }, + }), + ], + [ + "in-progress tool_call_update", + sessionUpdate({ + sessionUpdate: "tool_call_update", + toolCallId: "t1", + status: "in_progress", + rawInput: { command: "SECRET" }, + }), + ], + [ + "available_commands_update", + sessionUpdate({ + sessionUpdate: "available_commands_update", + availableCommands: [], + }), + ], + [ + "user prompt request", + makeEntry("session/prompt", { + prompt: [{ type: "text", text: "SECRET" }], + }), + ], + [ + "user_message", + makeEntry("_posthog/user_message", { message: "SECRET" }), + ], + ["unknown extension method", makeEntry("_posthog/some_new_event", {})], + ])("drops %s", (_name, entry) => { + expect(mapNotificationToLogRecord(entry)).toBeNull(); + }); + + it("caps body length", () => { + const mapped = mapNotificationToLogRecord( + makeEntry("_posthog/console", { + level: "info", + message: "x".repeat(5000), + }), + ); + + expect(mapped?.body.length).toBeLessThanOrEqual(2001); + }); + }); + + describe("logs export", () => { + let telemetry: OtelRunTelemetry; + + beforeEach(() => { + telemetry = new OtelRunTelemetry( + { + url: "https://us.i.posthog.com/i/v1/logs", + token: "phc_test_key", + flushIntervalMs: 100, + }, + RESOURCE, + ); + }); + + afterEach(async () => { + await telemetry.shutdown(); + }); + + it("pins run identity as resource attributes so runs are filterable per user", async () => { + telemetry.append(RUN_ID, makeEntry("_posthog/run_started", {})); + + await telemetry.flush(); + + const record = exportedLogs()[0]; + expect(record.resource.attributes).toMatchObject({ + "service.name": "posthog-code-agent", + "service.version": "1.2.3", + run_id: RUN_ID, + task_id: "task-123", + team_id: "42", + user_id: "7", + distinct_id: "distinct-1", + device_type: "cloud", + adapter: "claude", + run_mode: "background", + }); + // Without a traces URL, no spans are built and logs carry no trace ids. + expect(mockSpanExport).not.toHaveBeenCalled(); + expect(record.spanContext).toBeUndefined(); + }); + + it("never exports tool arguments, titles, or output content", async () => { + telemetry.append( + RUN_ID, + sessionUpdate({ + sessionUpdate: "tool_call", + toolCallId: "t1", + kind: "execute", + status: "pending", + title: "bash: cat .env", + rawInput: { command: "cat .env" }, + }), + ); + telemetry.append( + RUN_ID, + sessionUpdate({ + sessionUpdate: "tool_call_update", + toolCallId: "t1", + status: "completed", + rawOutput: { stdout: "AWS_KEY=leaked" }, + }), + ); + + await telemetry.flush(); + + const records = exportedLogs(); + expect(records).toHaveLength(2); + const surface = JSON.stringify( + records.map((record) => [record.body, record.attributes]), + ); + expect(surface).not.toContain(".env"); + expect(surface).not.toContain("leaked"); + }); + + it("ignores entries for other sessions", async () => { + telemetry.append("other-run", makeEntry("_posthog/run_started", {})); + + await telemetry.flush(); + + expect(mockLogExport).not.toHaveBeenCalled(); + }); + + it("swallows malformed entries instead of throwing", () => { + const junk = { + type: "notification", + timestamp: "not-a-date", + notification: { jsonrpc: "2.0", method: 123 }, + } as unknown as StoredNotification; + + expect(() => telemetry.append(RUN_ID, junk)).not.toThrow(); + }); + }); + + describe("trace export", () => { + let telemetry: OtelRunTelemetry; + + beforeEach(() => { + telemetry = new OtelRunTelemetry( + { + url: "https://us.i.posthog.com/i/v1/logs", + token: "phc_test_key", + tracesUrl: "https://us.i.posthog.com/i/v1/traces", + flushIntervalMs: 100, + }, + RESOURCE, + ); + }); + + function driveSuccessfulRun(): void { + telemetry.append(RUN_ID, makeEntry("_posthog/run_started", {})); + telemetry.append( + RUN_ID, + makeEntry("session/prompt", { + prompt: [{ type: "text", text: "SECRET" }], + }), + ); + telemetry.append( + RUN_ID, + sessionUpdate({ + sessionUpdate: "tool_call", + toolCallId: "t1", + kind: "execute", + status: "pending", + title: "bash: cat .env", + }), + ); + telemetry.append( + RUN_ID, + sessionUpdate({ + sessionUpdate: "tool_call_update", + toolCallId: "t1", + status: "completed", + }), + ); + telemetry.append( + RUN_ID, + makeEntry("_posthog/usage_update", { + used: { inputTokens: 10, outputTokens: 5 }, + cost: 0.1, + }), + ); + telemetry.append( + RUN_ID, + makeEntry("_posthog/turn_complete", { stopReason: "end_turn" }), + ); + telemetry.append(RUN_ID, makeEntry("_posthog/task_complete", {})); + } + + it("builds a run trace: root span, turn span, tool span", async () => { + driveSuccessfulRun(); + + await telemetry.shutdown(); + + const root = spanByName("task_run"); + const turn = spanByName("turn"); + const tool = spanByName("tool_call:execute"); + + expect(root.kind).toBe(SpanKind.SERVER); + expect(root.parentSpanContext).toBeUndefined(); + expect(turn.parentSpanContext?.spanId).toBe(root.spanContext().spanId); + expect(tool.parentSpanContext?.spanId).toBe(turn.spanContext().spanId); + + const traceId = root.spanContext().traceId; + expect(turn.spanContext().traceId).toBe(traceId); + expect(tool.spanContext().traceId).toBe(traceId); + + expect(root.status.code).toBe(SpanStatusCode.OK); + expect(turn.status.code).toBe(SpanStatusCode.OK); + expect(tool.status.code).toBe(SpanStatusCode.OK); + + expect(turn.attributes).toMatchObject({ + turn_index: 1, + stop_reason: "end_turn", + tokens_input: 10, + tokens_output: 5, + cost_usd: 0.1, + }); + expect(tool.attributes).toMatchObject({ + tool_kind: "execute", + tool_call_id: "t1", + tool_status: "completed", + }); + + // Same allowlist stance as logs: no prompt or tool content on spans. + const surface = JSON.stringify( + exportedSpans().map((span) => [span.name, span.attributes]), + ); + expect(surface).not.toContain("SECRET"); + expect(surface).not.toContain(".env"); + }); + + it("stamps log records with the span they belong to", async () => { + driveSuccessfulRun(); + + await telemetry.shutdown(); + + const root = spanByName("task_run"); + const tool = spanByName("tool_call:execute"); + const logs = exportedLogs(); + + const runStartedLog = logs.find((log) => log.body === "run started"); + const toolStartedLog = logs.find((log) => + log.body.startsWith("tool call started"), + ); + expect(runStartedLog?.spanContext?.spanId).toBe( + root.spanContext().spanId, + ); + expect(toolStartedLog?.spanContext?.spanId).toBe( + tool.spanContext().spanId, + ); + for (const log of logs) { + expect(log.spanContext?.traceId).toBe(root.spanContext().traceId); + } + }); + + it("marks failed tools, errored turns, and the errored run", async () => { + telemetry.append(RUN_ID, makeEntry("session/prompt", {})); + telemetry.append( + RUN_ID, + sessionUpdate({ + sessionUpdate: "tool_call", + toolCallId: "t1", + kind: "execute", + }), + ); + telemetry.append( + RUN_ID, + sessionUpdate({ + sessionUpdate: "tool_call_update", + toolCallId: "t1", + status: "failed", + }), + ); + telemetry.append( + RUN_ID, + makeEntry("_posthog/error", { + source: "agent_server", + error: "gateway exploded", + }), + ); + + await telemetry.shutdown(); + + expect(spanByName("tool_call:execute").status.code).toBe( + SpanStatusCode.ERROR, + ); + expect(spanByName("turn").status.code).toBe(SpanStatusCode.ERROR); + const root = spanByName("task_run"); + expect(root.status.code).toBe(SpanStatusCode.ERROR); + expect(root.status.message).toBe("gateway exploded"); + }); + + it("exports open spans on shutdown even without terminal events", async () => { + telemetry.append(RUN_ID, makeEntry("session/prompt", {})); + telemetry.append( + RUN_ID, + sessionUpdate({ + sessionUpdate: "tool_call", + toolCallId: "t1", + kind: "read", + }), + ); + + await telemetry.shutdown(); + + expect( + exportedSpans() + .map((span) => span.name) + .sort(), + ).toEqual(["task_run", "tool_call:read", "turn"]); + }); + }); +}); diff --git a/packages/agent/src/otel-telemetry.ts b/packages/agent/src/otel-telemetry.ts new file mode 100644 index 0000000000..0b009b829e --- /dev/null +++ b/packages/agent/src/otel-telemetry.ts @@ -0,0 +1,365 @@ +import { SeverityNumber } from "@opentelemetry/api-logs"; +import { OTLPLogExporter } from "@opentelemetry/exporter-logs-otlp-http"; +import { resourceFromAttributes } from "@opentelemetry/resources"; +import { + BatchLogRecordProcessor, + LoggerProvider, +} from "@opentelemetry/sdk-logs"; +import { + ATTR_SERVICE_NAME, + ATTR_SERVICE_VERSION, +} from "@opentelemetry/semantic-conventions"; +import { POSTHOG_NOTIFICATIONS } from "./acp-extensions"; +import { + type Attributes, + asRecord, + entryTime, + MAX_BODY_CHARS, + strAttr, + truncate, + usageAttributes, +} from "./otel-attributes"; +import { RunTraceBuilder } from "./otel-trace-builder"; +import type { SessionLogSink } from "./session-log-writer"; +import type { StoredNotification } from "./types"; +import type { Logger } from "./utils/logger"; + +const SERVICE_NAME = "posthog-code-agent"; +const DEFAULT_FLUSH_INTERVAL_MS = 2000; + +export interface OtelTelemetryConfig { + /** Full OTLP logs endpoint URL, e.g. "https://us.i.posthog.com/i/v1/logs" */ + url: string; + /** Project API key sent as a Bearer token */ + token: string; + /** Full OTLP traces endpoint URL; spans are off when unset */ + tracesUrl?: string; + /** Batch flush interval in ms (default: 2000) */ + flushIntervalMs?: number; +} + +/** + * Session identity pinned as OTel resource attributes. Resource attributes are + * indexed via resource_fingerprint in PostHog Logs, so runs are directly + * filterable per user/task/run in the Logs UI. + */ +export interface OtelSessionResource { + /** Parent task grouping - all runs for a task share this */ + taskId: string; + /** Primary conversation identifier - all events in a run share this */ + runId: string; + /** Deployment environment - "local" for desktop, "cloud" for cloud sandbox */ + deviceType: "local" | "cloud"; + teamId?: number; + userId?: number; + distinctId?: string; + /** Runtime adapter: "claude" or "codex" */ + adapter?: string; + /** Run mode: "interactive" or "background" */ + mode?: string; + agentVersion?: string; +} + +export interface MappedLogRecord { + severityNumber: SeverityNumber; + severityText: string; + body: string; + attributes: Attributes; +} + +const CONSOLE_SEVERITIES: Record = { + debug: [SeverityNumber.DEBUG, "DEBUG"], + info: [SeverityNumber.INFO, "INFO"], + warn: [SeverityNumber.WARN, "WARN"], + error: [SeverityNumber.ERROR, "ERROR"], +}; + +function record( + severity: [SeverityNumber, string], + body: string, + eventType: string, + attributes: Attributes = {}, +): MappedLogRecord { + return { + severityNumber: severity[0], + severityText: severity[1], + body: truncate(body, MAX_BODY_CHARS), + attributes: { event_type: eventType, ...attributes }, + }; +} + +const INFO = CONSOLE_SEVERITIES.info; +const WARN = CONSOLE_SEVERITIES.warn; +const ERROR = CONSOLE_SEVERITIES.error; + +function mapSessionUpdate( + method: string, + params: Record, +): MappedLogRecord | null { + const update = asRecord(params.update); + const updateType = update?.sessionUpdate; + if (!update || typeof updateType !== "string") return null; + + switch (updateType) { + case "tool_call": { + const attrs: Attributes = { session_update_type: updateType }; + strAttr(attrs, "tool_call_id", update.toolCallId); + const kind = strAttr(attrs, "tool_kind", update.kind); + strAttr(attrs, "tool_status", update.status ?? "pending"); + return record( + INFO, + `tool call started${kind ? ` (${kind})` : ""}`, + method, + attrs, + ); + } + case "tool_call_update": { + const status = update.status; + // In-progress snapshots re-send the growing tool input/output; only the + // terminal transition is run metadata. + if (status !== "completed" && status !== "failed") return null; + const attrs: Attributes = { session_update_type: updateType }; + strAttr(attrs, "tool_call_id", update.toolCallId); + strAttr(attrs, "tool_status", status); + return record( + status === "failed" ? WARN : INFO, + `tool call ${status}`, + method, + attrs, + ); + } + case "usage_update": + return record(INFO, "usage update", method, { + session_update_type: updateType, + ...usageAttributes(update), + }); + default: + return null; + } +} + +/** + * Maps a stored session notification to an exportable log record, or null for + * notification types that must not leave the sandbox. + * + * Allowlist by design: agent message/thought text, tool arguments, and tool + * output stay in the session log (the product source of truth) - only + * run-lifecycle metadata is exported, so prompts and repo content never reach + * the telemetry project. + */ +export function mapNotificationToLogRecord( + entry: StoredNotification, +): MappedLogRecord | null { + const rawMethod = entry.notification.method; + if (typeof rawMethod !== "string") return null; + // extNotification() can double-prefix custom methods (see matchesExt in + // acp-extensions.ts); normalize so both spellings map identically. + const method = rawMethod.startsWith("__posthog/") + ? rawMethod.slice(1) + : rawMethod; + const params = asRecord(entry.notification.params) ?? {}; + + if (method === "session/update") { + return mapSessionUpdate(method, params); + } + + switch (method) { + case POSTHOG_NOTIFICATIONS.RUN_STARTED: { + const attrs: Attributes = {}; + strAttr(attrs, "agent_version", params.agentVersion); + strAttr(attrs, "session_id", params.sessionId); + return record(INFO, "run started", method, attrs); + } + case POSTHOG_NOTIFICATIONS.SDK_SESSION: { + const attrs: Attributes = {}; + const adapter = strAttr(attrs, "adapter", params.adapter); + strAttr(attrs, "session_id", params.sessionId); + return record( + INFO, + `sdk session created${adapter ? ` (${adapter})` : ""}`, + method, + attrs, + ); + } + case POSTHOG_NOTIFICATIONS.USAGE_UPDATE: + return record(INFO, "usage update", method, usageAttributes(params)); + case POSTHOG_NOTIFICATIONS.TURN_COMPLETE: { + const attrs: Attributes = {}; + const stopReason = strAttr(attrs, "stop_reason", params.stopReason); + return record( + INFO, + `turn complete${stopReason ? ` (${stopReason})` : ""}`, + method, + attrs, + ); + } + case POSTHOG_NOTIFICATIONS.TASK_COMPLETE: { + const attrs: Attributes = {}; + strAttr(attrs, "stop_reason", params.stopReason); + return record(INFO, "task complete", method, attrs); + } + case POSTHOG_NOTIFICATIONS.ERROR: { + const attrs: Attributes = {}; + strAttr(attrs, "error_source", params.source); + strAttr(attrs, "stop_reason", params.stopReason); + const message = + typeof params.error === "string" ? params.error : "unknown error"; + return record(ERROR, `error: ${message}`, method, attrs); + } + case POSTHOG_NOTIFICATIONS.CONSOLE: { + const severity = + (typeof params.level === "string" && + CONSOLE_SEVERITIES[params.level]) || + INFO; + const message = + typeof params.message === "string" ? params.message : "console output"; + return record(severity, message, method, {}); + } + case POSTHOG_NOTIFICATIONS.PROGRESS: { + const attrs: Attributes = {}; + strAttr(attrs, "progress_group", params.group); + const step = strAttr(attrs, "progress_step", params.step); + const status = strAttr(attrs, "progress_status", params.status); + const label = typeof params.label === "string" ? params.label : undefined; + return record( + INFO, + `progress: ${step ?? "step"} ${status ?? ""}${label ? ` (${label})` : ""}`.trim(), + method, + attrs, + ); + } + case POSTHOG_NOTIFICATIONS.GIT_CHECKPOINT: { + const attrs: Attributes = {}; + strAttr(attrs, "branch", params.branch); + return record(INFO, "git checkpoint", method, attrs); + } + case POSTHOG_NOTIFICATIONS.BRANCH_CREATED: { + const attrs: Attributes = {}; + strAttr(attrs, "branch", params.branch ?? params.branchName); + return record(INFO, "branch created", method, attrs); + } + case POSTHOG_NOTIFICATIONS.MODE_CHANGE: { + const attrs: Attributes = {}; + strAttr(attrs, "run_mode", params.mode); + return record(INFO, "mode change", method, attrs); + } + case POSTHOG_NOTIFICATIONS.COMPACT_BOUNDARY: + return record(INFO, "compact boundary", method, {}); + case POSTHOG_NOTIFICATIONS.PERMISSION_REQUEST: + case POSTHOG_NOTIFICATIONS.PERMISSION_RESPONSE: + case POSTHOG_NOTIFICATIONS.PERMISSION_RESOLVED: { + // params.toolCall/options carry tool content; export identifiers only. + const attrs: Attributes = {}; + strAttr(attrs, "request_id", params.requestId); + strAttr(attrs, "tool_call_id", params.toolCallId); + const action = method.slice(method.indexOf("/") + 1).replace("_", " "); + return record(INFO, action, method, attrs); + } + default: + return null; + } +} + +/** + * Ships run telemetry to PostHog over OTLP: an allowlisted metadata subset of + * the session log to PostHog Logs (see mapNotificationToLogRecord) and, when a + * traces URL is configured, an APM trace per run (see RunTraceBuilder) with + * trace/span ids stamped on the log records so the two cross-link in the UI. + * Registered as a SessionLogWriter sink; the S3 session log remains the source + * of truth for the full transcript. + */ +export class OtelRunTelemetry implements SessionLogSink { + private loggerProvider: LoggerProvider; + private otelLogger: ReturnType; + private traceBuilder?: RunTraceBuilder; + private runId: string; + private debugLogger?: Logger; + private shutdownStarted = false; + + constructor( + config: OtelTelemetryConfig, + resource: OtelSessionResource, + debugLogger?: Logger, + ) { + this.runId = resource.runId; + this.debugLogger = debugLogger; + + const exporter = new OTLPLogExporter({ + url: config.url, + headers: { Authorization: `Bearer ${config.token}` }, + }); + + const processor = new BatchLogRecordProcessor(exporter, { + scheduledDelayMillis: config.flushIntervalMs ?? DEFAULT_FLUSH_INTERVAL_MS, + }); + + const resourceAttributes: Attributes = { + [ATTR_SERVICE_NAME]: SERVICE_NAME, + }; + strAttr(resourceAttributes, ATTR_SERVICE_VERSION, resource.agentVersion); + strAttr(resourceAttributes, "run_id", resource.runId); + strAttr(resourceAttributes, "task_id", resource.taskId); + strAttr(resourceAttributes, "device_type", resource.deviceType); + strAttr(resourceAttributes, "team_id", resource.teamId?.toString()); + strAttr(resourceAttributes, "user_id", resource.userId?.toString()); + strAttr(resourceAttributes, "distinct_id", resource.distinctId); + strAttr(resourceAttributes, "adapter", resource.adapter); + strAttr(resourceAttributes, "run_mode", resource.mode); + + const otelResource = resourceFromAttributes(resourceAttributes); + + this.loggerProvider = new LoggerProvider({ + resource: otelResource, + processors: [processor], + }); + + this.otelLogger = this.loggerProvider.getLogger("agent-session"); + + if (config.tracesUrl) { + this.traceBuilder = new RunTraceBuilder( + { + url: config.tracesUrl, + token: config.token, + flushIntervalMs: config.flushIntervalMs, + }, + otelResource, + ); + } + } + + append(sessionId: string, entry: StoredNotification): void { + // Resource attributes pin this writer to one run; ignore entries for any + // other session so records are never mislabeled. + if (sessionId !== this.runId || this.shutdownStarted) return; + try { + // The span state machine must see every entry: e.g. session/prompt is a + // turn boundary even though it never becomes a log record. + const context = this.traceBuilder?.handle(entry); + const mapped = mapNotificationToLogRecord(entry); + if (!mapped) return; + this.otelLogger.emit({ + ...mapped, + timestamp: entryTime(entry.timestamp), + context, + }); + } catch (error) { + // Telemetry must never interfere with the run. + this.debugLogger?.debug("Failed to emit OTel telemetry", { error }); + } + } + + async flush(): Promise { + await Promise.all([ + this.loggerProvider.forceFlush(), + this.traceBuilder?.flush(), + ]); + } + + /** Ends open spans, flushes batched records, then stops the providers. Idempotent. */ + async shutdown(): Promise { + if (this.shutdownStarted) return; + this.shutdownStarted = true; + await this.traceBuilder?.shutdown(); + await this.loggerProvider.shutdown(); + } +} diff --git a/packages/agent/src/otel-trace-builder.ts b/packages/agent/src/otel-trace-builder.ts new file mode 100644 index 0000000000..d3193e7689 --- /dev/null +++ b/packages/agent/src/otel-trace-builder.ts @@ -0,0 +1,274 @@ +import { + type Context, + ROOT_CONTEXT, + type Span, + SpanKind, + SpanStatusCode, + type Tracer, + trace, +} from "@opentelemetry/api"; +import { OTLPTraceExporter } from "@opentelemetry/exporter-trace-otlp-http"; +import type { Resource } from "@opentelemetry/resources"; +import { + BasicTracerProvider, + BatchSpanProcessor, +} from "@opentelemetry/sdk-trace-base"; +import { POSTHOG_NOTIFICATIONS } from "./acp-extensions"; +import { + type Attributes, + asRecord, + entryTime, + truncate, + usageAttributes, +} from "./otel-attributes"; +import type { StoredNotification } from "./types"; + +const DEFAULT_FLUSH_INTERVAL_MS = 2000; + +export interface RunTraceBuilderConfig { + /** Full OTLP traces endpoint URL, e.g. "https://us.i.posthog.com/i/v1/traces" */ + url: string; + /** Project API key sent as a Bearer token */ + token: string; + /** Batch flush interval in ms (default: 2000) */ + flushIntervalMs?: number; +} + +/** + * Builds one APM trace per run from the session notification stream: a root + * `task_run` span, a child `turn` span per prompt/turn, and a child + * `tool_call:` span per tool call. `handle()` returns the OTel context + * the corresponding log record should be emitted under, so logs and spans + * cross-link in the UI via trace_id/span_id. + * + * Spans carry the same allowlist stance as the log export: lifecycle, status, + * usage, and identifiers only — never prompts, tool arguments, or output. + */ +export class RunTraceBuilder { + private provider: BasicTracerProvider; + private tracer: Tracer; + private rootSpan: Span; + private rootContext: Context; + private rootErrored = false; + private turnSpan?: Span; + private turnContext?: Context; + private turnIndex = 0; + private toolSpans = new Map(); + private ended = false; + + constructor(config: RunTraceBuilderConfig, resource: Resource) { + const exporter = new OTLPTraceExporter({ + url: config.url, + headers: { Authorization: `Bearer ${config.token}` }, + }); + + this.provider = new BasicTracerProvider({ + resource, + spanProcessors: [ + new BatchSpanProcessor(exporter, { + scheduledDelayMillis: + config.flushIntervalMs ?? DEFAULT_FLUSH_INTERVAL_MS, + }), + ], + }); + + this.tracer = this.provider.getTracer("agent-session"); + this.rootSpan = this.tracer.startSpan("task_run", { + kind: SpanKind.SERVER, + }); + this.rootContext = trace.setSpan(ROOT_CONTEXT, this.rootSpan); + } + + /** + * Advances the span state machine with one stored entry and returns the + * context the entry's log record (if any) should attach to. + */ + handle(entry: StoredNotification): Context { + if (this.ended) return this.rootContext; + const rawMethod = entry.notification.method; + if (typeof rawMethod !== "string") return this.currentContext(); + const method = rawMethod.startsWith("__posthog/") + ? rawMethod.slice(1) + : rawMethod; + const params = asRecord(entry.notification.params) ?? {}; + const time = entryTime(entry.timestamp); + + switch (method) { + // The ACP prompt request is what starts a turn; its content never leaves + // the sandbox — it is only a turn boundary marker here. + case "session/prompt": + return this.startTurn(time); + case POSTHOG_NOTIFICATIONS.TURN_COMPLETE: + return this.endTurn(params, time); + case POSTHOG_NOTIFICATIONS.USAGE_UPDATE: { + (this.turnSpan ?? this.rootSpan).setAttributes(usageAttributes(params)); + return this.currentContext(); + } + case POSTHOG_NOTIFICATIONS.TASK_COMPLETE: + if (!this.rootErrored) { + this.rootSpan.setStatus({ code: SpanStatusCode.OK }); + } + return this.rootContext; + case POSTHOG_NOTIFICATIONS.ERROR: + return this.handleError(params, time); + case "session/update": + return this.handleSessionUpdate(params, time); + default: + return this.currentContext(); + } + } + + async flush(): Promise { + await this.provider.forceFlush(); + } + + /** Ends any open spans (status unset), then flushes and stops the provider. */ + async shutdown(): Promise { + if (this.ended) return; + this.ended = true; + const now = new Date(); + this.closeOpenTools(now); + this.closeTurn(undefined, now); + this.rootSpan.end(now); + await this.provider.shutdown(); + } + + private currentContext(): Context { + return this.turnContext ?? this.rootContext; + } + + private startTurn(time: Date): Context { + // A new prompt while a turn is still open means we missed its completion; + // close it without a status rather than nesting turns. + this.closeOpenTools(time); + this.closeTurn(undefined, time); + this.turnIndex += 1; + this.turnSpan = this.tracer.startSpan( + "turn", + { + kind: SpanKind.INTERNAL, + startTime: time, + attributes: { turn_index: this.turnIndex }, + }, + this.rootContext, + ); + this.turnContext = trace.setSpan(this.rootContext, this.turnSpan); + return this.turnContext; + } + + private endTurn(params: Record, time: Date): Context { + const context = this.turnContext ?? this.rootContext; + const stopReason = + typeof params.stopReason === "string" ? params.stopReason : undefined; + this.closeOpenTools(time); + this.closeTurn({ stopReason, errored: stopReason === "error" }, time); + return context; + } + + private closeTurn( + end: { stopReason?: string; errored?: boolean } | undefined, + time: Date, + ): void { + if (!this.turnSpan) return; + if (end?.stopReason) { + this.turnSpan.setAttribute("stop_reason", end.stopReason); + } + if (end) { + this.turnSpan.setStatus({ + code: end.errored ? SpanStatusCode.ERROR : SpanStatusCode.OK, + }); + } + this.turnSpan.end(time); + this.turnSpan = undefined; + this.turnContext = undefined; + } + + private handleSessionUpdate( + params: Record, + time: Date, + ): Context { + const update = asRecord(params.update); + const updateType = update?.sessionUpdate; + if (!update || typeof updateType !== "string") { + return this.currentContext(); + } + if (updateType === "tool_call") return this.startTool(update, time); + if (updateType === "tool_call_update") { + return this.handleToolUpdate(update, time); + } + if (updateType === "usage_update") { + (this.turnSpan ?? this.rootSpan).setAttributes(usageAttributes(update)); + } + return this.currentContext(); + } + + private startTool(update: Record, time: Date): Context { + const toolCallId = + typeof update.toolCallId === "string" ? update.toolCallId : undefined; + const existing = toolCallId ? this.toolSpans.get(toolCallId) : undefined; + if (existing) return existing.context; + + const kind = typeof update.kind === "string" ? update.kind : "unknown"; + const parentContext = this.turnContext ?? this.rootContext; + const attributes: Attributes = { tool_kind: kind }; + if (toolCallId) attributes.tool_call_id = toolCallId; + + const span = this.tracer.startSpan( + // Kind (read/edit/execute/...) is a small enum, so per-kind span names + // stay low-cardinality and make APM latency breakdowns useful. + `tool_call:${kind}`, + { kind: SpanKind.INTERNAL, startTime: time, attributes }, + parentContext, + ); + const context = trace.setSpan(parentContext, span); + if (toolCallId) { + this.toolSpans.set(toolCallId, { span, context }); + } else { + // Without an id there is no terminal update to match; record a marker. + span.end(time); + } + return context; + } + + private handleToolUpdate( + update: Record, + time: Date, + ): Context { + const toolCallId = + typeof update.toolCallId === "string" ? update.toolCallId : undefined; + const open = toolCallId ? this.toolSpans.get(toolCallId) : undefined; + if (!open || !toolCallId) return this.currentContext(); + + const status = update.status; + if (status === "completed" || status === "failed") { + open.span.setAttribute("tool_status", status); + open.span.setStatus({ + code: status === "failed" ? SpanStatusCode.ERROR : SpanStatusCode.OK, + }); + open.span.end(time); + this.toolSpans.delete(toolCallId); + } + return open.context; + } + + private handleError(params: Record, time: Date): Context { + this.rootErrored = true; + this.closeOpenTools(time); + this.closeTurn({ errored: true }, time); + this.rootSpan.setStatus({ + code: SpanStatusCode.ERROR, + message: + typeof params.error === "string" + ? truncate(params.error, 200) + : undefined, + }); + return this.rootContext; + } + + private closeOpenTools(time: Date): void { + for (const { span } of this.toolSpans.values()) { + span.end(time); + } + this.toolSpans.clear(); + } +} diff --git a/packages/agent/src/server/agent-server.ts b/packages/agent/src/server/agent-server.ts index 1053e4c2e2..bcfff7a20a 100644 --- a/packages/agent/src/server/agent-server.ts +++ b/packages/agent/src/server/agent-server.ts @@ -51,6 +51,7 @@ import { import type { PermissionMode } from "../execution-mode"; import { DEFAULT_CODEX_MODEL, fetchGatewayModels } from "../gateway-models"; import { HandoffCheckpointTracker } from "../handoff-checkpoint"; +import { OtelRunTelemetry } from "../otel-telemetry"; import { configurePersistentAgentState } from "../persistent-agent-state"; import { PostHogAPIClient } from "../posthog-api"; import { @@ -254,6 +255,8 @@ interface ActiveSession { sseController: SseController | null; deviceInfo: DeviceInfo; logWriter: SessionLogWriter; + /** Ships run telemetry (logs + spans) to PostHog; unset when the sandbox has no OTLP config */ + telemetry?: OtelRunTelemetry; /** Current permission mode, tracked for relay decisions */ permissionMode: PermissionMode; /** Whether a desktop client has ever connected via SSE during this session */ @@ -437,6 +440,47 @@ export class AgentServer { return payload.mode ?? this.config.mode; } + /** + * Ships run telemetry to PostHog when the sandbox provides an OTLP endpoint + * + token (POSTHOG_AGENT_OTEL_LOGS_URL/_TOKEN): metadata log records, plus an + * APM trace per run when POSTHOG_AGENT_OTEL_TRACES_URL is also set. Resource + * attributes carry the run/user identifiers so cloud runs are filterable per + * user, task, and run in the Logs UI. Returns undefined when unconfigured or + * on failure — telemetry must never block session startup. + */ + private createRunTelemetry( + payload: JwtPayload, + deviceInfo: DeviceInfo, + adapter: "claude" | "codex", + ): OtelRunTelemetry | undefined { + const { otelLogsUrl, otelLogsToken } = this.config; + if (!otelLogsUrl || !otelLogsToken) return undefined; + try { + return new OtelRunTelemetry( + { + url: otelLogsUrl, + token: otelLogsToken, + tracesUrl: this.config.otelTracesUrl, + }, + { + taskId: payload.task_id, + runId: payload.run_id, + deviceType: deviceInfo.type, + teamId: payload.team_id, + userId: payload.user_id, + distinctId: payload.distinct_id, + adapter, + mode: this.getEffectiveMode(payload), + agentVersion: this.config.version ?? packageJson.version, + }, + new Logger({ debug: false, prefix: "[OtelRunTelemetry]" }), + ); + } catch (error) { + this.logger.warn("Failed to initialize OTel run telemetry", error); + return undefined; + } + } + private getSessionPermissionMode(): PermissionMode { if (this.session?.permissionMode) { return this.session.permissionMode; @@ -1250,9 +1294,16 @@ export class AgentServer { userAgent: `posthog/cloud.hog.dev; version: ${this.config.version ?? packageJson.version}`, }); + const telemetry = this.createRunTelemetry( + payload, + deviceInfo, + runtimeAdapter, + ); + const logWriter = new SessionLogWriter({ posthogAPI, logger: new Logger({ debug: true, prefix: "[SessionLogWriter]" }), + sinks: telemetry ? [telemetry] : undefined, }); const acpConnection = createAcpConnection({ @@ -1425,6 +1476,7 @@ export class AgentServer { sseController, deviceInfo, logWriter, + telemetry, permissionMode: initialPermissionMode, hasDesktopConnected: sseController !== null, pendingHandoffGitState: undefined, @@ -3269,6 +3321,9 @@ ${signedCommitInstructions}${prLinkInstructions}${shellEfficiencyInstructions} } finally { await this.emitRtkSavings(); await this.eventStreamSender?.stop(); + // The sandbox can be torn down right after the failed status lands; + // don't leave the terminal record sitting in the OTel batch queue. + await this.session?.telemetry?.flush().catch(() => {}); } } @@ -3278,15 +3333,20 @@ ${signedCommitInstructions}${prLinkInstructions}${shellEfficiencyInstructions} | typeof POSTHOG_NOTIFICATIONS.ERROR, params: Record, ): void { - this.eventStreamSender?.enqueue({ - type: "notification", + const entry = { + type: "notification" as const, timestamp: new Date().toISOString(), notification: { - jsonrpc: "2.0", + jsonrpc: "2.0" as const, method, params, }, - }); + }; + this.eventStreamSender?.enqueue(entry); + // Terminal events bypass the SessionLogWriter (and its sinks), so mirror + // them onto the OTel writer directly — a failed run is exactly what the + // telemetry must record. + this.session?.telemetry?.append(this.session.payload.run_id, entry); } private configureEnvironment({ @@ -3853,6 +3913,15 @@ ${signedCommitInstructions}${prLinkInstructions}${shellEfficiencyInstructions} this.logger.error("Failed to flush session logs", error); } + // Shutdown ends open spans and flushes batched records; without it, + // sandbox teardown races the OTel batch delay and drops the tail of the + // run's telemetry. + try { + await this.session.telemetry?.shutdown(); + } catch (error) { + this.logger.error("Failed to shut down OTel run telemetry", error); + } + // Drain pending permissions before ACP cleanup to avoid deadlocks — // cleanup may await operations that are blocked on a permission response. for (const [, pending] of this.pendingPermissions) { diff --git a/packages/agent/src/server/bin.ts b/packages/agent/src/server/bin.ts index 46828f8e47..326c176f1a 100644 --- a/packages/agent/src/server/bin.ts +++ b/packages/agent/src/server/bin.ts @@ -48,6 +48,11 @@ const envSchema = z.object({ .enum(["true", "false"]) .transform((value) => value === "true") .optional(), + // OTLP pair for shipping run metadata to PostHog Logs; telemetry stays off + // unless both are set. The traces URL additionally enables APM spans. + POSTHOG_AGENT_OTEL_LOGS_URL: z.url().optional(), + POSTHOG_AGENT_OTEL_LOGS_TOKEN: z.string().min(1).optional(), + POSTHOG_AGENT_OTEL_TRACES_URL: z.url().optional(), }); const program = new Command(); @@ -183,6 +188,9 @@ program env.POSTHOG_TASK_RUN_EVENT_INGEST_STREAM_WINDOW_MS, eventIngestKeepStreamOpen: env.POSTHOG_TASK_RUN_EVENT_INGEST_KEEP_STREAM_OPEN, + otelLogsUrl: env.POSTHOG_AGENT_OTEL_LOGS_URL, + otelLogsToken: env.POSTHOG_AGENT_OTEL_LOGS_TOKEN, + otelTracesUrl: env.POSTHOG_AGENT_OTEL_TRACES_URL, repositoryPath: options.repositoryPath, repoReadyFile: options.repoReadyFile, apiUrl: env.POSTHOG_API_URL, diff --git a/packages/agent/src/server/types.ts b/packages/agent/src/server/types.ts index 24bdfdd5b2..8c664aac76 100644 --- a/packages/agent/src/server/types.ts +++ b/packages/agent/src/server/types.ts @@ -24,6 +24,12 @@ export interface AgentServerConfig { eventIngestBaseUrl?: string; eventIngestStreamWindowMs?: number; eventIngestKeepStreamOpen?: boolean; + /** Full OTLP logs URL for run telemetry, e.g. https://us.i.posthog.com/i/v1/logs */ + otelLogsUrl?: string; + /** Project API key for the OTLP logs/traces endpoints */ + otelLogsToken?: string; + /** Full OTLP traces URL for run spans, e.g. https://us.i.posthog.com/i/v1/traces */ + otelTracesUrl?: string; mode: AgentMode; taskId: string; runId: string; diff --git a/packages/agent/src/session-log-writer.test.ts b/packages/agent/src/session-log-writer.test.ts index efb2b91092..2a492b58ac 100644 --- a/packages/agent/src/session-log-writer.test.ts +++ b/packages/agent/src/session-log-writer.test.ts @@ -104,6 +104,52 @@ describe("SessionLogWriter", () => { }); }); + describe("sinks", () => { + it("delivers non-chunk entries to sinks and keeps persistence when a sink throws", async () => { + const goodSink = { append: vi.fn() }; + const badSink = { + append: vi.fn(() => { + throw new Error("sink down"); + }), + }; + const writer = new SessionLogWriter({ + posthogAPI: mockPosthogAPI, + sinks: [badSink, goodSink], + }); + writer.register("run-1", { taskId: "task-1", runId: "run-1" }); + + writer.appendRawLine( + "run-1", + makeSessionUpdate("agent_message_chunk", { + content: { type: "text", text: "chunk" }, + }), + ); + writer.appendRawLine( + "run-1", + JSON.stringify({ + jsonrpc: "2.0", + method: "_posthog/turn_complete", + params: { stopReason: "end_turn" }, + }), + ); + + // Buffered message chunks never reach sinks; the non-chunk entry does, + // even after the sink listed first has thrown. + expect(goodSink.append).toHaveBeenCalledTimes(1); + expect(goodSink.append.mock.calls[0][0]).toBe("run-1"); + expect(goodSink.append.mock.calls[0][1].notification.method).toBe( + "_posthog/turn_complete", + ); + + await writer.flush("run-1"); + + const persistedMethods = mockAppendLog.mock.calls + .flatMap((call) => call[2]) + .map((entry: StoredNotification) => entry.notification.method); + expect(persistedMethods).toContain("_posthog/turn_complete"); + }); + }); + describe("agent_message_chunk coalescing", () => { it("coalesces consecutive chunks into a single agent_message", async () => { const sessionId = "s1"; diff --git a/packages/agent/src/session-log-writer.ts b/packages/agent/src/session-log-writer.ts index 7941b8487e..a11566902c 100644 --- a/packages/agent/src/session-log-writer.ts +++ b/packages/agent/src/session-log-writer.ts @@ -2,12 +2,35 @@ import fs from "node:fs"; import fsp from "node:fs/promises"; import path from "node:path"; import { serializeError } from "@posthog/shared"; -import type { SessionContext } from "./otel-log-writer"; import type { PostHogAPIClient } from "./posthog-api"; import type { StoredNotification } from "./types"; import { isEmptyContentBlock } from "./utils/acp-content"; import { Logger } from "./utils/logger"; +/** + * Session context for a registered session. + * These are set once per session and shared by every persisted entry. + */ +export interface SessionContext { + /** Parent task grouping - all runs for a task share this */ + taskId: string; + /** Primary conversation identifier - all events in a run share this */ + runId: string; + /** Deployment environment - "local" for desktop, "cloud" for cloud sandbox */ + deviceType?: "local" | "cloud"; +} + +/** + * Receives every parsed non-chunk notification the writer persists, in + * arrival order. Streamed agent message/thought chunks are not delivered + * (neither raw nor coalesced) - sinks carry run metadata, not transcript + * content. Sink failures are swallowed so they can never break session log + * persistence. + */ +export interface SessionLogSink { + append(sessionId: string, entry: StoredNotification): void; +} + export interface SessionLogWriterOptions { /** PostHog API client for log persistence */ posthogAPI?: PostHogAPIClient; @@ -15,6 +38,8 @@ export interface SessionLogWriterOptions { logger?: Logger; /** Local cache path for instant log loading (e.g., ~/.posthog-code) */ localCachePath?: string; + /** Additional consumers of persisted entries (e.g. OTel telemetry) */ + sinks?: SessionLogSink[]; } interface ChunkBuffer { @@ -72,6 +97,8 @@ export class SessionLogWriter { private retryCounts: Map = new Map(); private sessions: Map = new Map(); private flushQueues: Map> = new Map(); + private sinks: SessionLogSink[]; + private warnedSinks: Set = new Set(); private logger: Logger; private localCachePath?: string; @@ -79,6 +106,7 @@ export class SessionLogWriter { constructor(options: SessionLogWriterOptions = {}) { this.posthogAPI = options.posthogAPI; this.localCachePath = options.localCachePath; + this.sinks = options.sinks ?? []; this.logger = options.logger ?? new Logger({ debug: false, prefix: "[SessionLogWriter]" }); @@ -187,6 +215,8 @@ export class SessionLogWriter { notification: message, }; + this.emitToSinks(sessionId, entry); + // Coalesce the local cache: buffer in-progress tool_call_update // snapshots (they re-send the full growing output) and write one merged // update per toolCallId. Written on a terminal update, any non-tool @@ -338,6 +368,24 @@ export class SessionLogWriter { } } + private emitToSinks(sessionId: string, entry: StoredNotification): void { + for (const sink of this.sinks) { + try { + sink.append(sessionId, entry); + } catch (error) { + // Warn once per sink: a broken sink at streaming rate would otherwise + // flood the console without ever affecting persistence. + if (!this.warnedSinks.has(sink)) { + this.warnedSinks.add(sink); + this.logger.warn( + "Session log sink failed; suppressing further errors from this sink", + { error: serializeError(error) }, + ); + } + } + } + } + private getUpdate( message: Record, ): Record | undefined { diff --git a/packages/agent/src/types.ts b/packages/agent/src/types.ts index c8d6a0ae0b..6e8e5a6cc8 100644 --- a/packages/agent/src/types.ts +++ b/packages/agent/src/types.ts @@ -77,19 +77,8 @@ export type OnLogCallback = ( data?: unknown, ) => void; -export interface OtelTransportConfig { - /** PostHog ingest host, e.g., "https://us.i.posthog.com" */ - host: string; - /** Project API key */ - apiKey: string; - /** Override the logs endpoint path (default: /i/v1/logs) */ - logsPath?: string; -} - export interface AgentConfig { posthog?: PostHogAPIConfig; - /** OTEL transport config for shipping logs to PostHog Logs */ - otelTransport?: OtelTransportConfig; /** Skip session log persistence (e.g. for preview sessions with no real task) */ skipLogPersistence?: boolean; /** Local cache path for instant log loading (e.g., ~/.posthog-code) */ diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 9321cf92e6..3faacde23a 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -294,7 +294,7 @@ importers: version: 4.6.2(react-dom@19.2.6(react@19.2.6))(react@19.2.6) react-scan: specifier: ^0.5.6 - version: 0.5.7(@opentelemetry/core@2.8.0(@opentelemetry/api@1.9.1))(esbuild@0.27.2)(eslint@10.5.0(jiti@2.7.0))(react-dom@19.2.6(react@19.2.6))(react@19.2.6)(rollup@4.57.1) + version: 0.5.7(@opentelemetry/core@2.8.0(@opentelemetry/api@1.9.1))(@opentelemetry/exporter-trace-otlp-http@0.208.0(@opentelemetry/api@1.9.1))(esbuild@0.27.2)(eslint@10.5.0(jiti@2.7.0))(react-dom@19.2.6(react@19.2.6))(react@19.2.6)(rollup@4.57.1) reflect-metadata: specifier: ^0.2.2 version: 0.2.2 @@ -703,18 +703,27 @@ importers: '@openai/codex': specifier: 0.140.0 version: 0.140.0 + '@opentelemetry/api': + specifier: ^1.9.1 + version: 1.9.1 '@opentelemetry/api-logs': specifier: ^0.208.0 version: 0.208.0 '@opentelemetry/exporter-logs-otlp-http': specifier: ^0.208.0 version: 0.208.0(@opentelemetry/api@1.9.1) + '@opentelemetry/exporter-trace-otlp-http': + specifier: ^0.208.0 + version: 0.208.0(@opentelemetry/api@1.9.1) '@opentelemetry/resources': specifier: ^2.0.0 version: 2.5.0(@opentelemetry/api@1.9.1) '@opentelemetry/sdk-logs': specifier: ^0.208.0 version: 0.208.0(@opentelemetry/api@1.9.1) + '@opentelemetry/sdk-trace-base': + specifier: ^2.8.0 + version: 2.8.0(@opentelemetry/api@1.9.1) '@opentelemetry/semantic-conventions': specifier: ^1.28.0 version: 1.39.0 @@ -2743,11 +2752,11 @@ packages: '@esbuild-kit/core-utils@3.3.2': resolution: {integrity: sha512-sPRAnw9CdSsRmEtnsl2WXWdyquogVpB3yZ3dgwJfe8zrOzTsV7cJvmwrKVa+0ma5BoiGJ+BoqkMvawbayKUsqQ==} - deprecated: 'Merged into tsx: https://tsx.is' + deprecated: 'Merged into tsx: https://tsx.hirok.io' '@esbuild-kit/esm-loader@2.6.5': resolution: {integrity: sha512-FxEMIkJKnodyA1OaCUoEvbYRkoZlLZ4d/eXFu9Fh8CbBBgP5EmZxrfTRyN0qpXZ4vOvqnE5YdRdcrmUUXuU+dA==} - deprecated: 'Merged into tsx: https://tsx.is' + deprecated: 'Merged into tsx: https://tsx.hirok.io' '@esbuild/aix-ppc64@0.25.12': resolution: {integrity: sha512-Hhmwd6CInZ3dwpuGTF8fJG6yoWmsToE+vYgD4nytZVxcu1ulHpUQRAB1UJ8+N1Am3Mz4+xOByoQoSZf4D+CpkA==} @@ -4287,6 +4296,12 @@ packages: peerDependencies: '@opentelemetry/api': ^1.3.0 + '@opentelemetry/exporter-trace-otlp-http@0.208.0': + resolution: {integrity: sha512-jbzDw1q+BkwKFq9yxhjAJ9rjKldbt5AgIy1gmEIJjEV/WRxQ3B6HcLVkwbjJ3RcMif86BDNKR846KJ0tY0aOJA==} + engines: {node: ^18.19.0 || >=20.6.0} + peerDependencies: + '@opentelemetry/api': ^1.3.0 + '@opentelemetry/instrumentation@0.214.0': resolution: {integrity: sha512-MHqEX5Dk59cqVah5LiARMACku7jXSVk9iVDWOea4x3cr7VfdByeDCURK6o1lntT1JS/Tsovw01UJrBhN3/uC5w==} engines: {node: ^18.19.0 || >=20.6.0} @@ -6328,8 +6343,8 @@ packages: '@types/react-dom': optional: true - '@react-grab/cli@0.1.47': - resolution: {integrity: sha512-Cc7d8mSwvoV8gpeTQbE8dMPdeXIyO6w+yIhzgi3jY06i03WLNhb/6jIxNBNF1cVRI7ujnFQXZA66BbnBNTpBSw==} + '@react-grab/cli@0.1.48': + resolution: {integrity: sha512-KXRZFN0b78BeVa4Tq1FC9kiXPpC5lS4pQp/mvQ1azy9dZUJ3zfc7Ei84+yvGh+WoYdceMCFxXfBp6qhU/G056g==} hasBin: true '@react-native-async-storage/async-storage@2.2.0': @@ -8361,6 +8376,11 @@ packages: peerDependencies: react: '>=17.0.1' + bippy@0.5.43: + resolution: {integrity: sha512-Tvu7b1M7+d8b9/YHaCeODEsi2CgbuoBql+dWSBrNnCuqJ1gMUeY3i0r+319hvjjl5GVBP6FFWxrKnq3fhZER0w==} + peerDependencies: + react: '>=17.0.1' + bl@4.1.0: resolution: {integrity: sha512-1W07cM9gS6DcLperZfFSj+bWLtaPGSOHWhPiGzXmvVJbRLdG82sH/Kn8EtW1VqWVA54AKf2h5k5BbnIbwF3h6w==} @@ -9043,8 +9063,8 @@ packages: resolution: {integrity: sha512-0je+qPKHEMohvfRTCEo3CrPG6cAzAYgmzKyxRiYSSDkS6eGJdyVJm7WaYA5ECaAD9wLB2T4EEeymA5aFVcYXCA==} engines: {node: '>=6'} - deslop-js@0.6.2: - resolution: {integrity: sha512-3+wV56jJd4zx6Mob2nQ4B8nIF4J12Xc2iCagNE9kdVTzMbNEIe99vwfaxrgdFC+RQ1NHXfjUkR3C4HVYnLLSfg==} + deslop-js@0.7.1: + resolution: {integrity: sha512-HsEoRI/bzuD0o2OVczYz42SXTCl5of3ax6eiojZbC/7gJsPNxxjPRvBysP88LXsHujYrIGJnGtFRnHwCmKWuxQ==} destroy@1.2.0: resolution: {integrity: sha512-2sJGJTaXIIaR1w4iJSNoN0hnMY7Gpc/n8D4qSCJw8QqFWXf7cuAgnEHxBpweaVcPevC2l3KpjYCx3NypQQgaJg==} @@ -12049,8 +12069,8 @@ packages: engines: {node: ^20.19.0 || >=22.12.0} hasBin: true - oxlint-plugin-react-doctor@0.6.2: - resolution: {integrity: sha512-8+YU8hwSPmS2dglPxLGRTSpaSHA5A2L0x7zU/7G+dp9V779jrpHiYMhJ5Ga9JUa0VtECHfpY+xUOYSt1IEP3YA==} + oxlint-plugin-react-doctor@0.7.1: + resolution: {integrity: sha512-fvARsCESDZYvDIlhuB/JlDeUhTOLHYstoDJCKm0pzh4HQQJVVV6gcrQajBjYo/hdHC1ukl7btKTK3rk4uZwuew==} engines: {node: ^20.19.0 || >=22.13.0} oxlint@1.66.0: @@ -12665,8 +12685,8 @@ packages: resolution: {integrity: sha512-+NRMYs2DyTP4/tqWz371Oo50JqmWltR1h2gcdgUMAWZJIAvrd0/SqlCfx7tpzpl/s36rzw6qH2MjoNrxtRNYhA==} engines: {node: ^20.9.0 || >=22} - react-doctor@0.6.2: - resolution: {integrity: sha512-uPFyoWhRvAG6vc7uLVbf2wg7ox0R8y+SgOAhAfAcs1soP4sIPwl4gEuWCNbAir/QIGcb5xrwFJvCM30WmgmeUg==} + react-doctor@0.7.1: + resolution: {integrity: sha512-Gmty7Enyrh6GPlz6Paq+UoL2O7YkTzNeHdflbqdp6fspX1UbUem5ejPyIUgo1jf77D6kB+INqsi2K+Mk/K8uBQ==} engines: {node: ^20.19.0 || >=22.13.0} hasBin: true @@ -12684,8 +12704,8 @@ packages: peerDependencies: react: '>=17.0.0' - react-grab@0.1.47: - resolution: {integrity: sha512-1GNy24KMJ4CY1IxorYO9mydItGi0L1HkQB19uYU3t0BMsJB0K+D/QYiaBz+rugRynyY8LzmXIuOcon1TykLlCg==} + react-grab@0.1.48: + resolution: {integrity: sha512-p3WnmK9LLvXE/c4ITPLlXcP1fkXo2VFEQqK94tIfcHIWKNdqdhYYFyNVioO50HR+uyHIwT63Z4txZDqJlVcD/Q==} hasBin: true peerDependencies: react: '>=17.0.0' @@ -18035,6 +18055,15 @@ snapshots: '@opentelemetry/otlp-transformer': 0.208.0(@opentelemetry/api@1.9.1) '@opentelemetry/sdk-logs': 0.208.0(@opentelemetry/api@1.9.1) + '@opentelemetry/exporter-trace-otlp-http@0.208.0(@opentelemetry/api@1.9.1)': + dependencies: + '@opentelemetry/api': 1.9.1 + '@opentelemetry/core': 2.2.0(@opentelemetry/api@1.9.1) + '@opentelemetry/otlp-exporter-base': 0.208.0(@opentelemetry/api@1.9.1) + '@opentelemetry/otlp-transformer': 0.208.0(@opentelemetry/api@1.9.1) + '@opentelemetry/resources': 2.2.0(@opentelemetry/api@1.9.1) + '@opentelemetry/sdk-trace-base': 2.2.0(@opentelemetry/api@1.9.1) + '@opentelemetry/instrumentation@0.214.0(@opentelemetry/api@1.9.1)': dependencies: '@opentelemetry/api': 1.9.1 @@ -18097,7 +18126,7 @@ snapshots: '@opentelemetry/api': 1.9.1 '@opentelemetry/core': 2.2.0(@opentelemetry/api@1.9.1) '@opentelemetry/resources': 2.2.0(@opentelemetry/api@1.9.1) - '@opentelemetry/semantic-conventions': 1.39.0 + '@opentelemetry/semantic-conventions': 1.41.1 '@opentelemetry/sdk-trace-base@2.8.0(@opentelemetry/api@1.9.1)': dependencies: @@ -19646,7 +19675,7 @@ snapshots: '@types/react': 19.2.17 '@types/react-dom': 19.2.3(@types/react@19.2.17) - '@react-grab/cli@0.1.47': + '@react-grab/cli@0.1.48': dependencies: agent-install: 0.0.6 commander: 14.0.3 @@ -19992,7 +20021,7 @@ snapshots: '@sentry/core@10.61.0': {} - '@sentry/node-core@10.61.0(@opentelemetry/api@1.9.1)(@opentelemetry/core@2.8.0(@opentelemetry/api@1.9.1))(@opentelemetry/instrumentation@0.214.0(@opentelemetry/api@1.9.1))(@opentelemetry/sdk-trace-base@2.8.0(@opentelemetry/api@1.9.1))': + '@sentry/node-core@10.61.0(@opentelemetry/api@1.9.1)(@opentelemetry/core@2.8.0(@opentelemetry/api@1.9.1))(@opentelemetry/exporter-trace-otlp-http@0.208.0(@opentelemetry/api@1.9.1))(@opentelemetry/instrumentation@0.214.0(@opentelemetry/api@1.9.1))(@opentelemetry/sdk-trace-base@2.8.0(@opentelemetry/api@1.9.1))': dependencies: '@sentry/conventions': 0.12.0 '@sentry/core': 10.61.0 @@ -20001,17 +20030,18 @@ snapshots: optionalDependencies: '@opentelemetry/api': 1.9.1 '@opentelemetry/core': 2.8.0(@opentelemetry/api@1.9.1) + '@opentelemetry/exporter-trace-otlp-http': 0.208.0(@opentelemetry/api@1.9.1) '@opentelemetry/instrumentation': 0.214.0(@opentelemetry/api@1.9.1) '@opentelemetry/sdk-trace-base': 2.8.0(@opentelemetry/api@1.9.1) - '@sentry/node@10.61.0(@opentelemetry/core@2.8.0(@opentelemetry/api@1.9.1))': + '@sentry/node@10.61.0(@opentelemetry/core@2.8.0(@opentelemetry/api@1.9.1))(@opentelemetry/exporter-trace-otlp-http@0.208.0(@opentelemetry/api@1.9.1))': dependencies: '@opentelemetry/api': 1.9.1 '@opentelemetry/instrumentation': 0.214.0(@opentelemetry/api@1.9.1) '@opentelemetry/sdk-trace-base': 2.8.0(@opentelemetry/api@1.9.1) '@opentelemetry/semantic-conventions': 1.41.1 '@sentry/core': 10.61.0 - '@sentry/node-core': 10.61.0(@opentelemetry/api@1.9.1)(@opentelemetry/core@2.8.0(@opentelemetry/api@1.9.1))(@opentelemetry/instrumentation@0.214.0(@opentelemetry/api@1.9.1))(@opentelemetry/sdk-trace-base@2.8.0(@opentelemetry/api@1.9.1)) + '@sentry/node-core': 10.61.0(@opentelemetry/api@1.9.1)(@opentelemetry/core@2.8.0(@opentelemetry/api@1.9.1))(@opentelemetry/exporter-trace-otlp-http@0.208.0(@opentelemetry/api@1.9.1))(@opentelemetry/instrumentation@0.214.0(@opentelemetry/api@1.9.1))(@opentelemetry/sdk-trace-base@2.8.0(@opentelemetry/api@1.9.1)) '@sentry/opentelemetry': 10.61.0(@opentelemetry/api@1.9.1)(@opentelemetry/core@2.8.0(@opentelemetry/api@1.9.1))(@opentelemetry/sdk-trace-base@2.8.0(@opentelemetry/api@1.9.1)) '@sentry/server-utils': 10.61.0 import-in-the-middle: 3.2.0 @@ -21348,7 +21378,7 @@ snapshots: sirv: 3.0.2 tinyglobby: 0.2.15 tinyrainbow: 3.1.0 - vitest: 4.1.8(@opentelemetry/api@1.9.0)(@types/node@22.20.0)(@vitest/ui@4.1.8)(jsdom@26.1.0)(msw@2.12.8(@types/node@22.20.0)(typescript@5.9.3))(vite@7.3.5(@types/node@22.20.0)(jiti@2.7.0)(lightningcss@1.32.0)(terser@5.46.0)(tsx@4.22.4)(yaml@2.9.0)) + vitest: 4.1.8(@opentelemetry/api@1.9.1)(@types/node@24.12.0)(@vitest/ui@4.1.8)(jsdom@26.1.0)(msw@2.12.8(@types/node@24.12.0)(typescript@5.9.3))(rolldown-vite@7.3.1(@emnapi/core@1.10.0)(@emnapi/runtime@1.10.0)(@types/node@24.12.0)(esbuild@0.27.2)(jiti@2.7.0)(terser@5.46.0)(tsx@4.22.4)(yaml@2.9.0)) '@vitest/utils@3.2.4': dependencies: @@ -21949,6 +21979,10 @@ snapshots: dependencies: react: 19.2.6 + bippy@0.5.43(react@19.2.6): + dependencies: + react: 19.2.6 + bl@4.1.0: dependencies: buffer: 5.7.1 @@ -22609,7 +22643,7 @@ snapshots: dequal@2.0.3: {} - deslop-js@0.6.2: + deslop-js@0.7.1: dependencies: '@oxc-project/types': 0.132.0 fast-glob: 3.3.3 @@ -26278,7 +26312,7 @@ snapshots: '@oxfmt/binding-win32-ia32-msvc': 0.45.0 '@oxfmt/binding-win32-x64-msvc': 0.45.0 - oxlint-plugin-react-doctor@0.6.2: + oxlint-plugin-react-doctor@0.7.1: dependencies: '@typescript-eslint/types': 8.62.0 eslint-scope: 9.1.2 @@ -26994,19 +27028,19 @@ snapshots: transitivePeerDependencies: - supports-color - react-doctor@0.6.2(@opentelemetry/core@2.8.0(@opentelemetry/api@1.9.1))(eslint@10.5.0(jiti@2.7.0)): + react-doctor@0.7.1(@opentelemetry/core@2.8.0(@opentelemetry/api@1.9.1))(@opentelemetry/exporter-trace-otlp-http@0.208.0(@opentelemetry/api@1.9.1))(eslint@10.5.0(jiti@2.7.0)): dependencies: '@babel/code-frame': 7.29.0 - '@sentry/node': 10.61.0(@opentelemetry/core@2.8.0(@opentelemetry/api@1.9.1)) + '@sentry/node': 10.61.0(@opentelemetry/core@2.8.0(@opentelemetry/api@1.9.1))(@opentelemetry/exporter-trace-otlp-http@0.208.0(@opentelemetry/api@1.9.1)) agent-install: 0.0.5 conf: 15.1.0 confbox: 0.2.4 - deslop-js: 0.6.2 + deslop-js: 0.7.1 eslint-plugin-react-hooks: 7.1.1(eslint@10.5.0(jiti@2.7.0)) jiti: 2.7.0 magicast: 0.5.3 oxlint: 1.66.0 - oxlint-plugin-react-doctor: 0.6.2 + oxlint-plugin-react-doctor: 0.7.1 prompts: 2.4.2 typescript: 5.9.3 vscode-languageserver: 9.0.1 @@ -27031,10 +27065,10 @@ snapshots: dependencies: react: 19.2.6 - react-grab@0.1.47(react@19.2.6): + react-grab@0.1.48(react@19.2.6): dependencies: - '@react-grab/cli': 0.1.47 - bippy: 0.5.42(react@19.2.6) + '@react-grab/cli': 0.1.48 + bippy: 0.5.43(react@19.2.6) optionalDependencies: react: 19.2.6 @@ -27264,7 +27298,7 @@ snapshots: react: 19.2.6 react-dom: 19.2.6(react@19.2.6) - react-scan@0.5.7(@opentelemetry/core@2.8.0(@opentelemetry/api@1.9.1))(esbuild@0.27.2)(eslint@10.5.0(jiti@2.7.0))(react-dom@19.2.6(react@19.2.6))(react@19.2.6)(rollup@4.57.1): + react-scan@0.5.7(@opentelemetry/core@2.8.0(@opentelemetry/api@1.9.1))(@opentelemetry/exporter-trace-otlp-http@0.208.0(@opentelemetry/api@1.9.1))(esbuild@0.27.2)(eslint@10.5.0(jiti@2.7.0))(react-dom@19.2.6(react@19.2.6))(react@19.2.6)(rollup@4.57.1): dependencies: '@babel/core': 7.29.0 '@babel/types': 7.29.7 @@ -27276,9 +27310,9 @@ snapshots: preact: 10.29.2 prompts: 2.4.2 react: 19.2.6 - react-doctor: 0.6.2(@opentelemetry/core@2.8.0(@opentelemetry/api@1.9.1))(eslint@10.5.0(jiti@2.7.0)) + react-doctor: 0.7.1(@opentelemetry/core@2.8.0(@opentelemetry/api@1.9.1))(@opentelemetry/exporter-trace-otlp-http@0.208.0(@opentelemetry/api@1.9.1))(eslint@10.5.0(jiti@2.7.0)) react-dom: 19.2.6(react@19.2.6) - react-grab: 0.1.47(react@19.2.6) + react-grab: 0.1.48(react@19.2.6) optionalDependencies: esbuild: 0.27.2 unplugin: 3.0.0 @@ -29037,6 +29071,36 @@ snapshots: transitivePeerDependencies: - msw + vitest@4.1.8(@opentelemetry/api@1.9.1)(@types/node@24.12.0)(@vitest/ui@4.1.8)(jsdom@26.1.0)(msw@2.12.8(@types/node@24.12.0)(typescript@5.9.3))(rolldown-vite@7.3.1(@emnapi/core@1.10.0)(@emnapi/runtime@1.10.0)(@types/node@24.12.0)(esbuild@0.27.2)(jiti@2.7.0)(terser@5.46.0)(tsx@4.22.4)(yaml@2.9.0)): + dependencies: + '@vitest/expect': 4.1.8 + '@vitest/mocker': 4.1.8(msw@2.12.8(@types/node@24.12.0)(typescript@5.9.3))(rolldown-vite@7.3.1(@emnapi/core@1.10.0)(@emnapi/runtime@1.10.0)(@types/node@24.12.0)(esbuild@0.27.2)(jiti@2.7.0)(terser@5.46.0)(tsx@4.22.4)(yaml@2.9.0)) + '@vitest/pretty-format': 4.1.8 + '@vitest/runner': 4.1.8 + '@vitest/snapshot': 4.1.8 + '@vitest/spy': 4.1.8 + '@vitest/utils': 4.1.8 + es-module-lexer: 2.0.0 + expect-type: 1.3.0 + magic-string: 0.30.21 + obug: 2.1.1 + pathe: 2.0.3 + picomatch: 4.0.3 + std-env: 4.1.0 + tinybench: 2.9.0 + tinyexec: 1.0.2 + tinyglobby: 0.2.15 + tinyrainbow: 3.1.0 + vite: rolldown-vite@7.3.1(@emnapi/core@1.10.0)(@emnapi/runtime@1.10.0)(@types/node@24.12.0)(esbuild@0.27.2)(jiti@2.7.0)(terser@5.46.0)(tsx@4.22.4)(yaml@2.9.0) + why-is-node-running: 2.3.0 + optionalDependencies: + '@opentelemetry/api': 1.9.1 + '@types/node': 24.12.0 + '@vitest/ui': 4.1.8(vitest@4.1.8) + jsdom: 26.1.0 + transitivePeerDependencies: + - msw + vitest@4.1.8(@opentelemetry/api@1.9.1)(@types/node@25.2.0)(@vitest/ui@4.1.8)(jsdom@26.1.0)(msw@2.12.8(@types/node@25.2.0)(typescript@5.9.3))(vite@7.3.5(@types/node@25.2.0)(jiti@2.7.0)(lightningcss@1.32.0)(terser@5.46.0)(tsx@4.21.0)(yaml@2.9.0)): dependencies: '@vitest/expect': 4.1.8 From 87f1a9fe9b821651415e944ab44c54e249a8ef6b Mon Sep 17 00:00:00 2001 From: Alessandro Pogliaghi Date: Mon, 6 Jul 2026 17:17:21 +0100 Subject: [PATCH 02/11] chore: add implementation report for agent run telemetry Generated-By: PostHog Code Task-Id: 0c511836-2180-455a-9b58-45df2a0661ec --- REPORT.md | 143 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 143 insertions(+) create mode 100644 REPORT.md diff --git a/REPORT.md b/REPORT.md new file mode 100644 index 0000000000..02e6c77f3c --- /dev/null +++ b/REPORT.md @@ -0,0 +1,143 @@ +# Agent run telemetry: PostHog Code cloud tasks → PostHog Logs + APM + +This branch is one half of a two-repo change; the companion branch is `posthog-code/agent-run-otel-telemetry` in `PostHog/posthog`. +This repo (`PostHog/code`) carries the telemetry emitter inside the agent; `PostHog/posthog` carries the configuration/injection side. + +## Goal + +Cloud task runs executed by PostHog Code should be observable in PostHog itself: +every run's lifecycle metadata delivered as OTel **logs** into the Logs product, and one OTel **trace** per run (root span, per-turn spans, per-tool-call spans) into APM, with logs and spans cross-linked via `trace_id`/`span_id`. + +Hard requirements: + +- **Filterable per user**: the Logs UI must answer "show me everything cloud runs did for this user" directly, so `user_id`/`distinct_id` (plus `team_id`, `task_id`, `run_id`) are OTel resource attributes, which PostHog Logs facets via `resource_fingerprint`. +- **Cloud tasks only** for now; desktop local runs do not export session telemetry. +- **Metadata only**: the S3 session log remains the source of truth for full transcripts. Telemetry never carries prompts, agent message/thought text, tool arguments, or tool output. + +## Background: what existed before + +- Agent session logs flow from the sandbox `agent-server` through `SessionLogWriter` to the Django endpoint `POST /api/projects/{team}/tasks/{task}/runs/{run}/append_log/`, which appends NDJSON to S3 (`TaskRun.append_log`, 30-day TTL). Two more delivery legs exist: an NDJSON event-ingest stream and SSE to connected clients. +- A February attempt at OTel logs export (`OtelLogWriter`, commits `6abadc79` → `99a3aea8` → `8876c8fb` in `PostHog/code`) was unwired, and its default endpoint `/i/v1/agent-logs` does not exist in the ingest service; it would 404 today. +- The correct ingest is the `capture-logs` Rust service (`rust/capture-logs/`): `POST /i/v1/logs` and `POST /i/v1/traces`, OTLP http/protobuf or http/JSON, auth `Authorization: Bearer `, 2 MB request cap, billed by uncompressed bytes, severity normalized to lowercase, prod host `https://us.i.posthog.com`. +- Working dogfood precedents followed here: the desktop Electron transport (`posthog-code-desktop` → `/i/v1/logs`), the engineering-analytics CI log emitter, the streamlit sandbox proxy (env-injected OTLP config), and the plugin-server metrics exporter (telemetry off unless URL + token are both set). + +## Architecture + +``` +sandbox (agentsh) +└── agent-server (PostHog/code, packages/agent) + ├── ACP streams (tapped) ──► SessionLogWriter ──► Django append_log ──► S3 (product log, unchanged) + │ │ + │ └─ sink ──► OtelRunTelemetry + │ ├─ log records ──► POST {POSTHOG_AGENT_OTEL_LOGS_URL} + │ └─ RunTraceBuilder spans ──► POST {POSTHOG_AGENT_OTEL_TRACES_URL} + └── terminal error events ──────────────────────► mirrored into OtelRunTelemetry directly + +capture-logs (Rust) ──► Kafka ──► ClickHouse (logs / trace_spans) ──► Logs + APM UI +``` + +Telemetry is emitted **from inside the sandbox** by the agent-server process, deliberately independent of the Django/S3 product path: if `append_log` is degraded, telemetry still flows, and a broken product log pipeline is exactly the failure telemetry must capture. +Egress works because `*.posthog.com` is in the agentsh `INFRASTRUCTURE_DOMAINS` allowlist (prod) and the new `SANDBOX_AGENT_OTEL_*_URL` hosts join the DEBUG-only firewall list (local dev). + +## What ships per run + +### Log records (service.name=posthog-code-agent) + +Resource attributes on every record: `service.name`, `service.version` (agent version), `run_id`, `task_id`, `team_id`, `user_id`, `distinct_id`, `device_type` (`cloud`), `adapter` (`claude`/`codex`), `run_mode` (`interactive`/`background`). + +Exported events (allowlist, everything else is dropped): + +| Event | Severity | Notable attributes | +| --- | --- | --- | +| `_posthog/run_started` | info | `agent_version`, `session_id` | +| `_posthog/sdk_session` | info | `adapter`, `session_id` | +| `_posthog/usage_update` (and the `session/update` variant) | info | `tokens_input/output/cached_read/cached_write`, `cost_usd` | +| `_posthog/turn_complete` | info | `stop_reason` | +| `_posthog/task_complete` | info | `stop_reason` | +| `_posthog/error` | **error** | `error_source`, `stop_reason`, message in body (capped) | +| `_posthog/console` | mapped from level | agent-server internal logs | +| `_posthog/progress` | info | `progress_group/step/status` | +| `_posthog/git_checkpoint`, `_posthog/branch_created` | info | `branch` | +| `_posthog/mode_change`, `_posthog/compact_boundary` | info | | +| `_posthog/permission_request/response/resolved` | info | `request_id`, `tool_call_id` (identifiers only; tool content excluded) | +| `session/update: tool_call` | info | `tool_call_id`, `tool_kind`, `tool_status` (no title, no rawInput) | +| `session/update: tool_call_update` (terminal only) | info / warn on `failed` | `tool_call_id`, `tool_status` | + +Deliberately dropped: `agent_message`, `agent_message_chunk`, `agent_thought_chunk`, `user_message`, `session/prompt` bodies, in-progress `tool_call_update` snapshots (they re-send the growing tool input/output), `available_commands_update`, and any unknown method (fail-closed allowlist). +Bodies are capped at 2000 chars; free-text attribute values at 200 chars (the `log_attributes` faceting table only indexes key/value pairs under 256 chars). + +### APM trace (one per run) + +- `task_run` root span (kind SERVER): opened at session init, closed at session cleanup; status OK on `task_complete`, ERROR with the error message on `_posthog/error`. +- `turn` child spans: opened on each ACP `session/prompt` (used purely as a boundary marker; its content is never read), closed on `_posthog/turn_complete`; attributes `turn_index`, `stop_reason`, plus the turn's token counts and `cost_usd` lifted from usage updates, so APM can rank slow or expensive turns directly. +- `tool_call:` grandchild spans (`execute`, `read`, `edit`, ...): opened on `tool_call`, closed on the terminal `tool_call_update`; status ERROR on `failed`; attributes `tool_call_id`, `tool_kind`, `tool_status`. Per-kind span names stay low-cardinality and make APM latency breakdowns by tool kind useful. +- Robustness: orphaned spans are closed (status unset) and exported at shutdown; a new prompt while a turn is open closes the stale turn; duplicate `tool_call` events are idempotent; an error cascades ERROR status through open tool and turn spans to the root. +- Every log record is emitted under the OTel context of the span it belongs to (tool logs on the tool span, lifecycle logs on the root), so `trace_id`/`span_id` land in the `logs` table columns and the UI links Logs ⇄ trace waterfall. + +### Delivery timing + +Telemetry is near-realtime, not end-of-turn: records are created the moment each notification flows through the writer and batched for at most 2 s (`BatchLogRecordProcessor` / `BatchSpanProcessor`, `scheduledDelayMillis` 2000). +Spans export when they end (tools mid-turn, turns at `turn_complete`, root at cleanup). +Flush safety nets: an explicit flush after a terminal error in `signalTaskComplete`, a full shutdown-flush in `cleanupSession` (which SIGTERM reaches via `stop()`), so sandbox teardown cannot eat the tail of a run's telemetry. + +## Changes in PostHog/code (this branch) + +- `packages/agent/src/otel-telemetry.ts` (renamed from `otel-log-writer.ts`): `OtelRunTelemetry`, the single `SessionLogSink` owning the OTLP log exporter and (when a traces URL is configured) the `RunTraceBuilder`. Contains the pure `mapNotificationToLogRecord()` allowlist mapper. Fixes the dead `/i/v1/agent-logs` default. Never throws into the run; ignores entries for other sessions. +- `packages/agent/src/otel-trace-builder.ts` (new): `RunTraceBuilder`, the span state machine described above; `handle(entry)` returns the context each log record should attach to. +- `packages/agent/src/otel-attributes.ts` (new): shared pure helpers (`strAttr`, `numAttr`, `usageAttributes`, truncation, caps). +- `packages/agent/src/session-log-writer.ts`: optional `sinks: SessionLogSink[]`, teed in `appendRawLine` after the entry is built; a throwing sink warns once and can never break product log persistence; message chunks never reach sinks. `SessionContext` moved here from the otel module. +- `packages/agent/src/server/agent-server.ts`: builds the telemetry per session from config (`createRunTelemetry`), passes it as the writer sink, stores it on the session, shuts it down in `cleanupSession`, flushes after terminal errors, and mirrors `enqueueTaskTerminalEvent` payloads into it directly (terminal `_posthog/error` events bypass `SessionLogWriter`, and a failed run is exactly what telemetry must record). +- `packages/agent/src/server/bin.ts` + `server/types.ts`: zod-validated env `POSTHOG_AGENT_OTEL_LOGS_URL`, `POSTHOG_AGENT_OTEL_LOGS_TOKEN`, `POSTHOG_AGENT_OTEL_TRACES_URL` → `AgentServerConfig.otelLogsUrl/otelLogsToken/otelTracesUrl`. Telemetry is off unless the logs pair is set; spans additionally require the traces URL (per-signal kill switch). +- `packages/agent/src/types.ts`: deleted the dead `OtelTransportConfig`/`AgentConfig.otelTransport` left over from the February attempt. +- Dependencies: `@opentelemetry/api`, `@opentelemetry/sdk-trace-base`, `@opentelemetry/exporter-trace-otlp-http`, version-aligned with the existing logs SDK (0.208.x experimental / 2.x stable line). +- Tests (71 passing): parameterized log-mapping matrix, a hard privacy test asserting exported payloads never contain tool args/titles/output, per-user resource attributes, session-mismatch guard, never-throws guard, sink isolation in `SessionLogWriter`, and four trace tests (span tree + statuses + attributes, log⇄span id correlation, error cascade, orphan export on shutdown). +- `packages/agent/README.md`: documents the env vars and behavior. + +## Changes in PostHog/posthog (companion branch) + +- `posthog/settings/temporal.py`: new optional settings `SANDBOX_AGENT_OTEL_LOGS_URL`, `SANDBOX_AGENT_OTEL_LOGS_TOKEN`, `SANDBOX_AGENT_OTEL_TRACES_URL` (all default unset = telemetry off). +- `products/tasks/backend/temporal/process_task/utils.py`: `get_sandbox_otel_env_vars()` maps those settings to the sandbox env vars, gated on the logs pair; called from **both** env assembly paths so fresh provisioning and snapshot-resume behave identically: + - `activities/provision_sandbox.py` `_build_environment_variables` + - `utils.py` `build_sandbox_environment_variables` (used by `create_sandbox_from_snapshot`) +- `products/tasks/backend/constants.py`: the three env keys added to `RESERVED_SANDBOX_ENVIRONMENT_VARIABLE_KEYS` so user-supplied SandboxEnvironment vars cannot override them. +- `products/tasks/backend/logic/services/agentsh.py`: `SANDBOX_AGENT_OTEL_LOGS_URL`/`SANDBOX_AGENT_OTEL_TRACES_URL` added to `_DEBUG_SANDBOX_URL_SETTINGS` so local-dev hosts pass the agentsh syscall firewall (prod egress already covered by `*.posthog.com`). +- `products/tasks/backend/logic/services/docker_sandbox.py`: `POSTHOG_AGENT_OTEL_LOGS_URL`/`POSTHOG_AGENT_OTEL_TRACES_URL` added to `_DOCKER_URL_ENV_KEYS` so localhost URLs are rewritten to `host.docker.internal` for local Docker sandboxes. +- Tests: parameterized gating matrix on `_build_environment_variables` (5 rows: full config, logs-only, partial configs, traces-without-logs all correctly gated) and a `SimpleTestCase` wiring guard on the snapshot-resume path. +- `docs/internal/sandboxes-setup-guide.md`: local-dev setup section for the new settings. + +## Key design decisions + +1. **Emit from the sandbox, not from Django.** Independence from the product log path (see Architecture), matching the streamlit sandbox precedent. The Django-tee alternative would add an outbound call to a hot API path and go dark precisely when `append_log` breaks. +2. **`POSTHOG_`-prefixed env vars instead of standard `OTEL_*` names.** The sandbox env is inherited by the user's own processes (their tests, their apps). Standard `OTEL_EXPORTER_OTLP_*` vars would make any OTel SDK in user code silently auto-export the user's telemetry into our internal project. Custom names mean only agent-server reads the config. +3. **`service.name=posthog-code-agent`, not `posthog-code`.** `service.name` identifies the emitting process, not the product: the desktop app already ships its process logs as `posthog-code-desktop`, and `service_name` is both the primary Logs UI facet and part of the ClickHouse sort key `(team_id, service_name, timestamp)`, so component-level names keep streams separable and queries narrow. House pattern matches (`posthog-django-*`, `node-*`, `github-ci-logs`). +4. **Fail-closed allowlist for content.** Only known lifecycle events are exported; unknown methods are dropped. This is a privacy boundary (customer prompts/repo content must not reach the telemetry project) and a cost control (logs are billed by bytes; in-progress tool snapshots re-send growing output). +5. **Generic `SessionLogSink` instead of hardcoding OTel into `SessionLogWriter`.** The February attempt was removed partly because of hard coupling; the sink interface keeps the writer single-purpose, is desktop-neutral (no sinks wired there), and isolates sink failures. +6. **Terminal-error mirror.** `enqueueTaskTerminalEvent` feeds only the event-ingest stream, bypassing `SessionLogWriter`; without the explicit mirror the most important record (run failed) would be missing from telemetry. +7. **Per-signal kill switch.** Logs and spans have separate URLs; unsetting the traces URL disables spans without touching logs, and unsetting either of the logs pair disables everything. +8. **Token exposure is acceptable by design.** The sandbox receives a project API key of the telemetry project: a write-only, public-by-design key class (the same class that ships in client SDKs), far weaker than the `POSTHOG_PERSONAL_API_KEY` already present in the sandbox. Worst case is junk telemetry writes; `capture-logs` has a token drop list as the kill switch. + +## Verification + +- `PostHog/code`: 71 tests pass in the agent package (including the new telemetry suite), `tsc --noEmit` clean via turbo, biome clean on all touched files (one pre-existing warning untouched). The package's pre-existing test failures in this environment (missing Postgres/git fixtures) were confirmed byte-identical with and without these changes by running the failing files against a stashed tree. +- `PostHog/posthog`: 21 tests pass across `test_provision_sandbox.py` and the new `TestBuildSandboxEnvironmentVariables`; `ruff check`/`format` clean on all touched files. DB-dependent suites in this sandbox fail identically with and without the change (no Postgres available). +- Local-dev routing verified: Caddy serves `/i/v1/logs`/`/i/v1/traces` on `localhost:8000` and proxies to `capture-logs`, and the Docker URL rewrite covers the new vars. + +## Rollout runbook (what remains) + +1. Choose the destination telemetry project and create/locate its project API key. Recommendation: the shared internal project where `posthog-code-desktop` logs and Code analytics already land, so desktop and cloud correlate in one Logs view (separable by `service_name`). +2. Set in prod US: + - `SANDBOX_AGENT_OTEL_LOGS_URL=https://us.i.posthog.com/i/v1/logs` + - `SANDBOX_AGENT_OTEL_LOGS_TOKEN=` + - `SANDBOX_AGENT_OTEL_TRACES_URL=https://us.i.posthog.com/i/v1/traces` +3. Run one cloud task; verify in the destination project: Logs filtered by `service.name=posthog-code-agent` (facet by `distinct_id`/`user_id`/`run_id`), and the APM trace for the run (`task_run` → `turn` → `tool_call:*` waterfall, logs linked from spans). +4. Add a saved Logs view and alerts (error severity on the service; volume anomaly), and watch billed bytes for a week; the event allowlist and body caps are the tuning knobs. +5. Optional follow-ups: unify the desktop transport with the new telemetry module; consider a shared `product` resource attribute across `posthog-code-*` services; sampling if volume warrants. + +## Configuration reference + +| Where | Name | Meaning | +| --- | --- | --- | +| Django settings | `SANDBOX_AGENT_OTEL_LOGS_URL` | Full OTLP logs ingest URL; unset = telemetry off | +| Django settings | `SANDBOX_AGENT_OTEL_LOGS_TOKEN` | Project API key of the telemetry project; unset = telemetry off | +| Django settings | `SANDBOX_AGENT_OTEL_TRACES_URL` | Full OTLP traces ingest URL; unset = spans off, logs unaffected | +| Sandbox env (injected) | `POSTHOG_AGENT_OTEL_LOGS_URL` / `_TOKEN` / `POSTHOG_AGENT_OTEL_TRACES_URL` | Read by `agent-server` (`bin.ts`); reserved keys, not user-overridable | From daaa91fbb1270f0572690a986c56e8c3865dca84 Mon Sep 17 00:00:00 2001 From: Alessandro Pogliaghi Date: Mon, 6 Jul 2026 17:49:19 +0100 Subject: [PATCH 03/11] fix(agent): mark successful runs ok and mirror fatal crashes into telemetry Addresses three review findings: - The sandbox never emits task_complete for successful runs (the terminal "completed" status is decided by the workflow outside), so the root span was ending with unset status on success. The trace builder now marks the root span OK when a turn ends cleanly with end_turn; a run error still always wins and a turn completion arriving after an error cannot flip the status back. - reportFatalError (uncaught exception / unhandled rejection) marked the run failed via the API but never touched telemetry, leaving hard crashes invisible in the new pipeline. It now mirrors an error record (error_source=agent_server_crash) and shuts telemetry down, ending the root span as errored and flushing before the process dies. - The README "Writing logs" section still described the removed otel-log-writer as the preferred full-transcript path to the nonexistent /i/v1/agent-logs endpoint; rewritten to match the sink-based metadata export. Generated-By: PostHog Code Task-Id: 0c511836-2180-455a-9b58-45df2a0661ec --- packages/agent/README.md | 7 ++---- packages/agent/src/otel-telemetry.test.ts | 10 ++++++++- packages/agent/src/otel-trace-builder.ts | 11 +++++++++ packages/agent/src/server/agent-server.ts | 27 +++++++++++++++++++++++ 4 files changed, 49 insertions(+), 6 deletions(-) diff --git a/packages/agent/README.md b/packages/agent/README.md index 71247db686..4943731edd 100644 --- a/packages/agent/README.md +++ b/packages/agent/README.md @@ -212,12 +212,9 @@ Logs serve two purposes: real-time observability and session resume. Every ACP m ### Writing logs -`SessionLogWriter` (`src/session-log-writer.ts`) is a per-session multiplexer that buffers raw ndJson lines. On flush (auto-scheduled 500ms after writes, or explicit), it dispatches to whichever backend is configured: +`SessionLogWriter` (`src/session-log-writer.ts`) is a per-session multiplexer that buffers raw ndJson lines. On flush (auto-scheduled 500ms after writes, or explicit), it POSTs batched `StoredNotification` entries to the Django API via `PostHogAPIClient.appendTaskRunLog()`; the API stores them in S3 as the task run's `log_url`. This S3 log is the source of truth for the full transcript and for session resume. -- **OTEL** (`src/otel-log-writer.ts`) — preferred path. Creates an OpenTelemetry `LoggerProvider` per session with resource attributes (`task_id`, `run_id`, `device_type`) set once and indexed via `resource_fingerprint`. Each ndJson line is emitted as an OTEL log record with an `event_type` attribute (the ACP method name) and exported via OTLP HTTP to PostHog's `/i/v1/agent-logs` endpoint. Batch flush interval defaults to 500ms. -- **Legacy S3** — falls back to `PostHogAPIClient.appendTaskRunLog()`, which POSTs batched `StoredNotification` entries to the Django API. The API stores them as the task run's `log_url`. - -Both backends can be active simultaneously — OTEL for fast indexed queries, S3 for full log download. +Independently of that flush cycle, the writer tees every parsed non-chunk entry to optional `SessionLogSink`s at append time. In cloud, `OtelRunTelemetry` (`src/otel-telemetry.ts`) is wired as a sink and exports an allowlisted metadata subset (run/turn/tool lifecycle, usage, errors — never message content or tool arguments) to PostHog Logs, plus an APM trace per run when configured. See "Optional run telemetry" above for the env vars and behavior. Sink failures can never break S3 log persistence. ### Resuming from logs diff --git a/packages/agent/src/otel-telemetry.test.ts b/packages/agent/src/otel-telemetry.test.ts index eba2c86a54..121b3bd09d 100644 --- a/packages/agent/src/otel-telemetry.test.ts +++ b/packages/agent/src/otel-telemetry.test.ts @@ -483,7 +483,9 @@ describe("OtelRunTelemetry", () => { RUN_ID, makeEntry("_posthog/turn_complete", { stopReason: "end_turn" }), ); - telemetry.append(RUN_ID, makeEntry("_posthog/task_complete", {})); + // Deliberately no task_complete: production never emits it (the + // terminal "completed" status is decided outside the sandbox), so the + // root span's OK must come from the clean end_turn above. } it("builds a run trace: root span, turn span, tool span", async () => { @@ -578,6 +580,12 @@ describe("OtelRunTelemetry", () => { error: "gateway exploded", }), ); + // A turn completion arriving after the error must not flip the run + // back to OK. + telemetry.append( + RUN_ID, + makeEntry("_posthog/turn_complete", { stopReason: "end_turn" }), + ); await telemetry.shutdown(); diff --git a/packages/agent/src/otel-trace-builder.ts b/packages/agent/src/otel-trace-builder.ts index d3193e7689..5fc1d37740 100644 --- a/packages/agent/src/otel-trace-builder.ts +++ b/packages/agent/src/otel-trace-builder.ts @@ -41,6 +41,10 @@ export interface RunTraceBuilderConfig { * the corresponding log record should be emitted under, so logs and spans * cross-link in the UI via trace_id/span_id. * + * Root span status: OK once a turn ends cleanly (`end_turn`) or on + * task_complete, ERROR on a run error (which always wins), unset when the run + * ends without either (cancelled / timed out). + * * Spans carry the same allowlist stance as the log export: lifecycle, status, * usage, and identifiers only — never prompts, tool arguments, or output. */ @@ -162,6 +166,13 @@ export class RunTraceBuilder { typeof params.stopReason === "string" ? params.stopReason : undefined; this.closeOpenTools(time); this.closeTurn({ stopReason, errored: stopReason === "error" }, time); + // The sandbox never emits task_complete for successful runs (the terminal + // "completed" status is decided by the workflow outside), so a cleanly + // finished turn is the success signal for the run. A later error still + // wins via rootErrored/handleError. + if (stopReason === "end_turn" && !this.rootErrored) { + this.rootSpan.setStatus({ code: SpanStatusCode.OK }); + } return context; } diff --git a/packages/agent/src/server/agent-server.ts b/packages/agent/src/server/agent-server.ts index bcfff7a20a..ff0bf79592 100644 --- a/packages/agent/src/server/agent-server.ts +++ b/packages/agent/src/server/agent-server.ts @@ -885,6 +885,33 @@ export class AgentServer { stopError, ); } + + // Mirror the crash into run telemetry and shut it down (ends the root + // span as errored and flushes) - the process is about to die, so nothing + // else will get this record out. + try { + const session = this.session; + if (session?.telemetry) { + session.telemetry.append(session.payload.run_id, { + type: "notification", + timestamp: new Date().toISOString(), + notification: { + jsonrpc: "2.0", + method: POSTHOG_NOTIFICATIONS.ERROR, + params: { + source: "agent_server_crash", + error: `Agent server crashed: ${errorMessage}`, + }, + }, + }); + await session.telemetry.shutdown(); + } + } catch (telemetryError) { + this.logger.error( + "Failed to flush telemetry after fatal error", + telemetryError, + ); + } } private authenticateRequest( From 16f62788763200a806842104fd08ed047e6c39c1 Mon Sep 17 00:00:00 2001 From: Alessandro Pogliaghi Date: Mon, 6 Jul 2026 17:49:29 +0100 Subject: [PATCH 04/11] chore: sync implementation report with review fixes Generated-By: PostHog Code Task-Id: 0c511836-2180-455a-9b58-45df2a0661ec --- REPORT.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/REPORT.md b/REPORT.md index 02e6c77f3c..87d58f7f19 100644 --- a/REPORT.md +++ b/REPORT.md @@ -68,7 +68,7 @@ Bodies are capped at 2000 chars; free-text attribute values at 200 chars (the `l ### APM trace (one per run) -- `task_run` root span (kind SERVER): opened at session init, closed at session cleanup; status OK on `task_complete`, ERROR with the error message on `_posthog/error`. +- `task_run` root span (kind SERVER): opened at session init, closed at session cleanup. Status OK once a turn ends cleanly with `end_turn` (the sandbox never emits `task_complete` for successful runs; the terminal "completed" status is decided by the workflow outside, so a clean turn end is the in-sandbox success signal), ERROR with the error message on `_posthog/error` (an error always wins, a later turn completion cannot flip it back), unset when the run ends without either (cancelled or timed out). - `turn` child spans: opened on each ACP `session/prompt` (used purely as a boundary marker; its content is never read), closed on `_posthog/turn_complete`; attributes `turn_index`, `stop_reason`, plus the turn's token counts and `cost_usd` lifted from usage updates, so APM can rank slow or expensive turns directly. - `tool_call:` grandchild spans (`execute`, `read`, `edit`, ...): opened on `tool_call`, closed on the terminal `tool_call_update`; status ERROR on `failed`; attributes `tool_call_id`, `tool_kind`, `tool_status`. Per-kind span names stay low-cardinality and make APM latency breakdowns by tool kind useful. - Robustness: orphaned spans are closed (status unset) and exported at shutdown; a new prompt while a turn is open closes the stale turn; duplicate `tool_call` events are idempotent; an error cascades ERROR status through open tool and turn spans to the root. @@ -86,7 +86,7 @@ Flush safety nets: an explicit flush after a terminal error in `signalTaskComple - `packages/agent/src/otel-trace-builder.ts` (new): `RunTraceBuilder`, the span state machine described above; `handle(entry)` returns the context each log record should attach to. - `packages/agent/src/otel-attributes.ts` (new): shared pure helpers (`strAttr`, `numAttr`, `usageAttributes`, truncation, caps). - `packages/agent/src/session-log-writer.ts`: optional `sinks: SessionLogSink[]`, teed in `appendRawLine` after the entry is built; a throwing sink warns once and can never break product log persistence; message chunks never reach sinks. `SessionContext` moved here from the otel module. -- `packages/agent/src/server/agent-server.ts`: builds the telemetry per session from config (`createRunTelemetry`), passes it as the writer sink, stores it on the session, shuts it down in `cleanupSession`, flushes after terminal errors, and mirrors `enqueueTaskTerminalEvent` payloads into it directly (terminal `_posthog/error` events bypass `SessionLogWriter`, and a failed run is exactly what telemetry must record). +- `packages/agent/src/server/agent-server.ts`: builds the telemetry per session from config (`createRunTelemetry`), passes it as the writer sink, stores it on the session, shuts it down in `cleanupSession`, flushes after terminal errors, and mirrors `enqueueTaskTerminalEvent` payloads into it directly (terminal `_posthog/error` events bypass `SessionLogWriter`, and a failed run is exactly what telemetry must record). Fatal crashes (`reportFatalError`, the uncaught-exception/unhandled-rejection path) also mirror an error record (`error_source=agent_server_crash`) and shut telemetry down, so hard process deaths reach the telemetry project instead of vanishing. - `packages/agent/src/server/bin.ts` + `server/types.ts`: zod-validated env `POSTHOG_AGENT_OTEL_LOGS_URL`, `POSTHOG_AGENT_OTEL_LOGS_TOKEN`, `POSTHOG_AGENT_OTEL_TRACES_URL` → `AgentServerConfig.otelLogsUrl/otelLogsToken/otelTracesUrl`. Telemetry is off unless the logs pair is set; spans additionally require the traces URL (per-signal kill switch). - `packages/agent/src/types.ts`: deleted the dead `OtelTransportConfig`/`AgentConfig.otelTransport` left over from the February attempt. - Dependencies: `@opentelemetry/api`, `@opentelemetry/sdk-trace-base`, `@opentelemetry/exporter-trace-otlp-http`, version-aligned with the existing logs SDK (0.208.x experimental / 2.x stable line). @@ -112,7 +112,7 @@ Flush safety nets: an explicit flush after a terminal error in `signalTaskComple 3. **`service.name=posthog-code-agent`, not `posthog-code`.** `service.name` identifies the emitting process, not the product: the desktop app already ships its process logs as `posthog-code-desktop`, and `service_name` is both the primary Logs UI facet and part of the ClickHouse sort key `(team_id, service_name, timestamp)`, so component-level names keep streams separable and queries narrow. House pattern matches (`posthog-django-*`, `node-*`, `github-ci-logs`). 4. **Fail-closed allowlist for content.** Only known lifecycle events are exported; unknown methods are dropped. This is a privacy boundary (customer prompts/repo content must not reach the telemetry project) and a cost control (logs are billed by bytes; in-progress tool snapshots re-send growing output). 5. **Generic `SessionLogSink` instead of hardcoding OTel into `SessionLogWriter`.** The February attempt was removed partly because of hard coupling; the sink interface keeps the writer single-purpose, is desktop-neutral (no sinks wired there), and isolates sink failures. -6. **Terminal-error mirror.** `enqueueTaskTerminalEvent` feeds only the event-ingest stream, bypassing `SessionLogWriter`; without the explicit mirror the most important record (run failed) would be missing from telemetry. +6. **Terminal-error mirrors.** Two paths bypass `SessionLogWriter` and are mirrored into telemetry explicitly: `enqueueTaskTerminalEvent` (agent-server-sourced run errors, which feed only the event-ingest stream) and `reportFatalError` (unrecoverable crashes, which mark the run failed via the API with no session log involvement). Without the mirrors the most important records, failed and crashed runs, would be missing from telemetry. 7. **Per-signal kill switch.** Logs and spans have separate URLs; unsetting the traces URL disables spans without touching logs, and unsetting either of the logs pair disables everything. 8. **Token exposure is acceptable by design.** The sandbox receives a project API key of the telemetry project: a write-only, public-by-design key class (the same class that ships in client SDKs), far weaker than the `POSTHOG_PERSONAL_API_KEY` already present in the sandbox. Worst case is junk telemetry writes; `capture-logs` has a token drop list as the kill switch. From 33b7e88309d1f3791378eab37ede4539300e9911 Mon Sep 17 00:00:00 2001 From: Alessandro Pogliaghi Date: Tue, 7 Jul 2026 09:43:54 +0100 Subject: [PATCH 05/11] 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 --- packages/agent/src/otel-telemetry.test.ts | 55 +++++++++++++++-------- packages/agent/src/otel-telemetry.ts | 27 ++++------- packages/agent/src/otel-trace-builder.ts | 42 +++++++++++------ 3 files changed, 74 insertions(+), 50 deletions(-) diff --git a/packages/agent/src/otel-telemetry.test.ts b/packages/agent/src/otel-telemetry.test.ts index 121b3bd09d..456b40a689 100644 --- a/packages/agent/src/otel-telemetry.test.ts +++ b/packages/agent/src/otel-telemetry.test.ts @@ -170,21 +170,6 @@ describe("OtelRunTelemetry", () => { body: "error: boom", attrs: { error_source: "agent_server" }, }, - { - name: "console warn", - entry: makeEntry("_posthog/console", { - level: "warn", - message: "careful", - }), - severityText: "WARN", - body: "careful", - }, - { - name: "console with unknown level", - entry: makeEntry("_posthog/console", { level: "silly", message: "m" }), - severityText: "INFO", - body: "m", - }, { name: "progress", entry: makeEntry("_posthog/progress", { @@ -321,6 +306,19 @@ describe("OtelRunTelemetry", () => { "user_message", makeEntry("_posthog/user_message", { message: "SECRET" }), ], + // Console lines are free-text agent-server diagnostics; some interpolate + // content (e.g. the prompt preview logged on user_message handling). + [ + "console with interpolated prompt preview", + makeEntry("_posthog/console", { + level: "debug", + message: "Processing user message (detectedPrUrl=none): SECRET...", + }), + ], + [ + "console error", + makeEntry("_posthog/console", { level: "error", message: "SECRET" }), + ], ["unknown extension method", makeEntry("_posthog/some_new_event", {})], ])("drops %s", (_name, entry) => { expect(mapNotificationToLogRecord(entry)).toBeNull(); @@ -328,9 +326,9 @@ describe("OtelRunTelemetry", () => { it("caps body length", () => { const mapped = mapNotificationToLogRecord( - makeEntry("_posthog/console", { - level: "info", - message: "x".repeat(5000), + makeEntry("_posthog/error", { + source: "agent_server", + error: "x".repeat(5000), }), ); @@ -531,6 +529,27 @@ describe("OtelRunTelemetry", () => { expect(surface).not.toContain(".env"); }); + it("does not leave root OK when a later turn ends non-clean", async () => { + driveSuccessfulRun(); + // Second turn gets cancelled: the latest outcome wins, so the earlier + // clean turn must not leave the run marked OK. + telemetry.append( + RUN_ID, + makeEntry("session/prompt", { + prompt: [{ type: "text", text: "again" }], + }), + ); + telemetry.append( + RUN_ID, + makeEntry("_posthog/turn_complete", { stopReason: "cancelled" }), + ); + + await telemetry.shutdown(); + + expect(spanByName("task_run").status.code).toBe(SpanStatusCode.UNSET); + expect(exportedSpans().filter((s) => s.name === "turn")).toHaveLength(2); + }); + it("stamps log records with the span they belong to", async () => { driveSuccessfulRun(); diff --git a/packages/agent/src/otel-telemetry.ts b/packages/agent/src/otel-telemetry.ts index 0b009b829e..c0ce887cb3 100644 --- a/packages/agent/src/otel-telemetry.ts +++ b/packages/agent/src/otel-telemetry.ts @@ -67,13 +67,6 @@ export interface MappedLogRecord { attributes: Attributes; } -const CONSOLE_SEVERITIES: Record = { - debug: [SeverityNumber.DEBUG, "DEBUG"], - info: [SeverityNumber.INFO, "INFO"], - warn: [SeverityNumber.WARN, "WARN"], - error: [SeverityNumber.ERROR, "ERROR"], -}; - function record( severity: [SeverityNumber, string], body: string, @@ -88,9 +81,9 @@ function record( }; } -const INFO = CONSOLE_SEVERITIES.info; -const WARN = CONSOLE_SEVERITIES.warn; -const ERROR = CONSOLE_SEVERITIES.error; +const INFO: [SeverityNumber, string] = [SeverityNumber.INFO, "INFO"]; +const WARN: [SeverityNumber, string] = [SeverityNumber.WARN, "WARN"]; +const ERROR: [SeverityNumber, string] = [SeverityNumber.ERROR, "ERROR"]; function mapSessionUpdate( method: string, @@ -206,15 +199,11 @@ export function mapNotificationToLogRecord( typeof params.error === "string" ? params.error : "unknown error"; return record(ERROR, `error: ${message}`, method, attrs); } - case POSTHOG_NOTIFICATIONS.CONSOLE: { - const severity = - (typeof params.level === "string" && - CONSOLE_SEVERITIES[params.level]) || - INFO; - const message = - typeof params.message === "string" ? params.message : "console output"; - return record(severity, message, method, {}); - } + // POSTHOG_NOTIFICATIONS.CONSOLE is deliberately NOT exported: those are + // free-text agent-server diagnostics that interpolate arbitrary data + // (prompt previews, stringified extension params), so shipping them would + // leak content the allowlist exists to keep in the sandbox. They remain + // in the S3 session log and the event-ingest stream. case POSTHOG_NOTIFICATIONS.PROGRESS: { const attrs: Attributes = {}; strAttr(attrs, "progress_group", params.group); diff --git a/packages/agent/src/otel-trace-builder.ts b/packages/agent/src/otel-trace-builder.ts index 5fc1d37740..6f4872b1a5 100644 --- a/packages/agent/src/otel-trace-builder.ts +++ b/packages/agent/src/otel-trace-builder.ts @@ -41,9 +41,12 @@ export interface RunTraceBuilderConfig { * the corresponding log record should be emitted under, so logs and spans * cross-link in the UI via trace_id/span_id. * - * Root span status: OK once a turn ends cleanly (`end_turn`) or on - * task_complete, ERROR on a run error (which always wins), unset when the run - * ends without either (cancelled / timed out). + * Root span status is resolved at shutdown from the LATEST turn outcome (the + * root span only exports when it ends, so earlier turns must not leave a + * sticky OK): OK when the last turn ended cleanly (`end_turn`, or an explicit + * task_complete), ERROR on a run error (which always wins) or a last turn + * that stopped with `error`, unset otherwise (cancelled / refused / timed out + * / no completed turns). * * Spans carry the same allowlist stance as the log export: lifecycle, status, * usage, and identifiers only — never prompts, tool arguments, or output. @@ -54,6 +57,8 @@ export class RunTraceBuilder { private rootSpan: Span; private rootContext: Context; private rootErrored = false; + /** stopReason of the most recent completed turn; drives root status at shutdown */ + private lastStopReason?: string; private turnSpan?: Span; private turnContext?: Context; private turnIndex = 0; @@ -109,9 +114,9 @@ export class RunTraceBuilder { return this.currentContext(); } case POSTHOG_NOTIFICATIONS.TASK_COMPLETE: - if (!this.rootErrored) { - this.rootSpan.setStatus({ code: SpanStatusCode.OK }); - } + // Explicit success signal (forward-compat; production decides the + // terminal status outside the sandbox) — treated as a clean outcome. + this.lastStopReason = "end_turn"; return this.rootContext; case POSTHOG_NOTIFICATIONS.ERROR: return this.handleError(params, time); @@ -126,13 +131,25 @@ export class RunTraceBuilder { await this.provider.forceFlush(); } - /** Ends any open spans (status unset), then flushes and stops the provider. */ + /** + * Ends any open spans (status unset), resolves the root status from the + * latest turn outcome, then flushes and stops the provider. + */ async shutdown(): Promise { if (this.ended) return; this.ended = true; const now = new Date(); this.closeOpenTools(now); this.closeTurn(undefined, now); + if (!this.rootErrored) { + if (this.lastStopReason === "end_turn") { + this.rootSpan.setStatus({ code: SpanStatusCode.OK }); + } else if (this.lastStopReason === "error") { + this.rootSpan.setStatus({ code: SpanStatusCode.ERROR }); + } + // Any other latest outcome (cancelled, refusal, max_tokens, none) + // leaves the status unset: neither success nor failure. + } this.rootSpan.end(now); await this.provider.shutdown(); } @@ -167,12 +184,11 @@ export class RunTraceBuilder { this.closeOpenTools(time); this.closeTurn({ stopReason, errored: stopReason === "error" }, time); // The sandbox never emits task_complete for successful runs (the terminal - // "completed" status is decided by the workflow outside), so a cleanly - // finished turn is the success signal for the run. A later error still - // wins via rootErrored/handleError. - if (stopReason === "end_turn" && !this.rootErrored) { - this.rootSpan.setStatus({ code: SpanStatusCode.OK }); - } + // "completed" status is decided by the workflow outside), so the latest + // turn outcome is the run's success signal — recorded here, resolved into + // the root span status at shutdown so an early clean turn can't leave a + // stale OK on a run whose last turn was cancelled. + this.lastStopReason = stopReason; return context; } From ed14102813be240e43a93ea36714262a1096e265 Mon Sep 17 00:00:00 2001 From: Alessandro Pogliaghi Date: Tue, 7 Jul 2026 09:44:06 +0100 Subject: [PATCH 06/11] chore: sync implementation report with console-export and root-status fixes Generated-By: PostHog Code Task-Id: 0c511836-2180-455a-9b58-45df2a0661ec --- REPORT.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/REPORT.md b/REPORT.md index 87d58f7f19..0d414c32d8 100644 --- a/REPORT.md +++ b/REPORT.md @@ -55,7 +55,6 @@ Exported events (allowlist, everything else is dropped): | `_posthog/turn_complete` | info | `stop_reason` | | `_posthog/task_complete` | info | `stop_reason` | | `_posthog/error` | **error** | `error_source`, `stop_reason`, message in body (capped) | -| `_posthog/console` | mapped from level | agent-server internal logs | | `_posthog/progress` | info | `progress_group/step/status` | | `_posthog/git_checkpoint`, `_posthog/branch_created` | info | `branch` | | `_posthog/mode_change`, `_posthog/compact_boundary` | info | | @@ -63,12 +62,12 @@ Exported events (allowlist, everything else is dropped): | `session/update: tool_call` | info | `tool_call_id`, `tool_kind`, `tool_status` (no title, no rawInput) | | `session/update: tool_call_update` (terminal only) | info / warn on `failed` | `tool_call_id`, `tool_status` | -Deliberately dropped: `agent_message`, `agent_message_chunk`, `agent_thought_chunk`, `user_message`, `session/prompt` bodies, in-progress `tool_call_update` snapshots (they re-send the growing tool input/output), `available_commands_update`, and any unknown method (fail-closed allowlist). +Deliberately dropped: `agent_message`, `agent_message_chunk`, `agent_thought_chunk`, `user_message`, `session/prompt` bodies, in-progress `tool_call_update` snapshots (they re-send the growing tool input/output), `available_commands_update`, `_posthog/console` (free-text agent-server diagnostics interpolate arbitrary data — e.g. the prompt preview logged on user-message handling and stringified extension params — so exporting them would leak content; they stay in the S3 log and event-ingest stream), and any unknown method (fail-closed allowlist). Bodies are capped at 2000 chars; free-text attribute values at 200 chars (the `log_attributes` faceting table only indexes key/value pairs under 256 chars). ### APM trace (one per run) -- `task_run` root span (kind SERVER): opened at session init, closed at session cleanup. Status OK once a turn ends cleanly with `end_turn` (the sandbox never emits `task_complete` for successful runs; the terminal "completed" status is decided by the workflow outside, so a clean turn end is the in-sandbox success signal), ERROR with the error message on `_posthog/error` (an error always wins, a later turn completion cannot flip it back), unset when the run ends without either (cancelled or timed out). +- `task_run` root span (kind SERVER): opened at session init, closed at session cleanup. Status is resolved at shutdown from the latest turn outcome (the sandbox never emits `task_complete` for successful runs — the terminal "completed" status is decided by the workflow outside — so the last turn is the in-sandbox success signal): OK when the last turn ended with `end_turn`, ERROR with the error message on `_posthog/error` (an error always wins, later turn completions cannot flip it back) or when the last turn stopped with `error`, unset otherwise (cancelled / refused / timed out / no completed turns). Resolving at shutdown rather than per-turn means an early clean turn cannot leave a stale OK on a run whose final turn was cancelled. - `turn` child spans: opened on each ACP `session/prompt` (used purely as a boundary marker; its content is never read), closed on `_posthog/turn_complete`; attributes `turn_index`, `stop_reason`, plus the turn's token counts and `cost_usd` lifted from usage updates, so APM can rank slow or expensive turns directly. - `tool_call:` grandchild spans (`execute`, `read`, `edit`, ...): opened on `tool_call`, closed on the terminal `tool_call_update`; status ERROR on `failed`; attributes `tool_call_id`, `tool_kind`, `tool_status`. Per-kind span names stay low-cardinality and make APM latency breakdowns by tool kind useful. - Robustness: orphaned spans are closed (status unset) and exported at shutdown; a new prompt while a turn is open closes the stale turn; duplicate `tool_call` events are idempotent; an error cascades ERROR status through open tool and turn spans to the root. From 213b0fb323f599651ccc67740a365ac68a09e415 Mon Sep 17 00:00:00 2001 From: Alessandro Pogliaghi Date: Tue, 7 Jul 2026 14:51:37 +0100 Subject: [PATCH 07/11] fix(aio): harden cloud run terminal state --- packages/agent/tsup.config.ts | 1 + packages/core/src/sessions/sessionService.ts | 190 +++++++-- packages/core/src/sessions/sessionStore.ts | 12 +- .../src/sessions/sessionViewState.test.ts | 87 ++++ .../core/src/sessions/sessionViewState.ts | 22 +- .../src/task-detail/cloudRunState.test.ts | 51 +++ .../core/src/task-detail/cloudRunState.ts | 17 +- .../sessions/sessionServiceHost.test.ts | 379 ++++++++++++++++++ .../features/sessions/sessionStore.test.ts | 41 ++ .../task-detail/hooks/useCloudRunState.ts | 9 +- .../features/task-detail/hooks/useTaskData.ts | 9 +- .../ui/src/features/tasks/queries.test.ts | 18 + packages/ui/src/features/tasks/queries.ts | 4 + .../src/features/tasks/taskFreshness.test.ts | 65 +++ .../ui/src/features/tasks/taskFreshness.ts | 32 ++ .../src/router/routes/code/tasks/$taskId.tsx | 19 +- .../website/$channelId/tasks/$taskId.tsx | 49 ++- 17 files changed, 953 insertions(+), 52 deletions(-) create mode 100644 packages/core/src/sessions/sessionViewState.test.ts create mode 100644 packages/core/src/task-detail/cloudRunState.test.ts create mode 100644 packages/ui/src/features/tasks/queries.test.ts create mode 100644 packages/ui/src/features/tasks/taskFreshness.test.ts create mode 100644 packages/ui/src/features/tasks/taskFreshness.ts diff --git a/packages/agent/tsup.config.ts b/packages/agent/tsup.config.ts index 3e54f6d317..db9b69a113 100644 --- a/packages/agent/tsup.config.ts +++ b/packages/agent/tsup.config.ts @@ -87,6 +87,7 @@ const sharedOptions = { "@posthog/shared", "@posthog/git", "@posthog/enricher", + /^@opentelemetry\//, "fflate", ], external: [ diff --git a/packages/core/src/sessions/sessionService.ts b/packages/core/src/sessions/sessionService.ts index a0c12d0dda..d0c72e327a 100644 --- a/packages/core/src/sessions/sessionService.ts +++ b/packages/core/src/sessions/sessionService.ts @@ -591,6 +591,7 @@ export class SessionService { private scheduledCloudQueueFlushes = new Set(); private cloudRunIdleTracker: CloudRunIdleTracker; private nextCloudTaskWatchToken = 0; + private hydratingCloudSessions = new Set(); private supersededRunIds = new Set(); private subscriptions = new Map< string, @@ -4122,6 +4123,24 @@ export class SessionService { if (onStatusChange) { existingWatcher.onStatusChange = onStatusChange; } + if (isTerminalStatus(runStatus)) { + const existing = this.d.store.getSessionByTaskId(taskId); + if (existing?.taskRunId === taskRunId) { + this.hydrateCloudTaskSessionFromLogs( + taskId, + taskRunId, + logUrl, + taskDescription, + runStatus, + runState, + ); + } + this.finalizeTerminalCloudTask(taskId, taskRunId, { + status: runStatus, + }); + this.stopCloudTaskWatch(taskId); + return () => {}; + } // Ensure configOptions is populated on revisit const existing = this.d.store.getSessionByTaskId(taskId); if (existing) { @@ -4261,7 +4280,7 @@ export class SessionService { initialReasoningEffort, ); - if (shouldHydrateSession) { + if (shouldHydrateSession || isTerminalStatus(runStatus)) { this.hydrateCloudTaskSessionFromLogs( taskId, taskRunId, @@ -4272,18 +4291,30 @@ export class SessionService { ); } + if (isTerminalStatus(runStatus)) { + this.finalizeTerminalCloudTask(taskId, taskRunId, { + status: runStatus, + }); + return () => {}; + } + // Subscribe before starting the main-process watcher so the first replayed // SSE/log burst cannot race ahead of the renderer subscription. const subscription = this.d.trpc.cloudTask.onUpdate.subscribe( { taskId, runId }, { onData: (update: CloudTaskUpdatePayload) => { + const isStaleNonTerminalStatus = this.isStaleNonTerminalCloudUpdate( + taskRunId, + update, + ); this.handleCloudTaskUpdate(taskRunId, update); const watcher = this.cloudTaskWatchers.get(taskId); if ( (update.kind === "status" || update.kind === "snapshot" || update.kind === "error") && + !isStaleNonTerminalStatus && watcher?.onStatusChange ) { watcher.onStatusChange(); @@ -4356,11 +4387,23 @@ export class SessionService { runStatus?: TaskRunStatus, runState?: Record, ): void { + const isTerminalRun = isTerminalStatus(runStatus); + const isResumeRun = Boolean(runState?.resume_from_run_id); + const hydrationMode = isTerminalRun + ? "terminal-chain" + : isResumeRun + ? "resume-chain" + : "single"; + const hydrationKey = `${taskId}:${taskRunId}:${hydrationMode}`; + if (this.hydratingCloudSessions.has(hydrationKey)) { + return; + } + this.hydratingCloudSessions.add(hydrationKey); + void (async () => { let rawEntries: StoredLogEntry[]; let totalLineCount: number; - const isResumeRun = Boolean(runState?.resume_from_run_id); - if (isTerminalStatus(runStatus) || isResumeRun) { + if (isTerminalRun || isResumeRun) { // Resume chains need the full history even while the leaf run is still // active; otherwise a renderer restart hydrates only the final run. // Non-resume in-progress runs keep using the single-run log so hydrate @@ -4384,6 +4427,13 @@ export class SessionService { return; } totalLineCount = rawEntries.length; + if (rawEntries.length === 0 && logUrl) { + const parsed = await this.fetchSessionLogs(logUrl, taskRunId); + if (parsed.rawEntries.length > 0) { + rawEntries = parsed.rawEntries; + totalLineCount = parsed.totalLineCount; + } + } } else { const parsed = await this.fetchSessionLogs(logUrl, taskRunId); rawEntries = parsed.rawEntries; @@ -4395,6 +4445,25 @@ export class SessionService { return; } + if (rawEntries.length === 0) { + if (isTerminalRun) { + this.initialCloudOptimisticPrompt.delete(taskId); + this.d.store.clearTailOptimisticItems(taskRunId); + } else { + const seedContent = + this.initialCloudOptimisticPrompt.get(taskId) ?? taskDescription; + if (seedContent?.trim()) { + this.d.store.appendOptimisticItem(taskRunId, { + type: "user_message", + content: seedContent, + timestamp: Date.now(), + }); + } + } + this.pendingPermissionHydratedRuns.add(taskRunId); + return; + } + const events = convertStoredEntriesToEvents(rawEntries); const hasUserPrompt = events.some( (e: AcpMessage) => @@ -4409,7 +4478,7 @@ export class SessionService { // its chip renders right away) over the bare task description. const seedContent = this.initialCloudOptimisticPrompt.get(taskId) ?? taskDescription; - if (!hasUserPrompt && seedContent?.trim()) { + if (!isTerminalRun && !hasUserPrompt && seedContent?.trim()) { this.d.store.appendOptimisticItem(taskRunId, { type: "user_message", content: seedContent, @@ -4421,15 +4490,21 @@ export class SessionService { this.initialCloudOptimisticPrompt.delete(taskId); this.d.store.clearTailOptimisticItems(taskRunId); } - - if (rawEntries.length === 0) { - this.pendingPermissionHydratedRuns.add(taskRunId); - return; + if (isTerminalRun) { + this.initialCloudOptimisticPrompt.delete(taskId); + this.d.store.clearTailOptimisticItems(taskRunId); } // If live updates already populated a processed count, don't overwrite // that newer state with the persisted baseline fetched during startup. - if ( + // Terminal hydration is different: it is the final transcript, so apply + // it when the persisted chain has more lines than the local stream. + const effectiveLineCount = Math.max(totalLineCount, rawEntries.length); + if (isTerminalRun) { + if ((session.processedLineCount ?? 0) >= effectiveLineCount) { + return; + } + } else if ( session.processedLineCount !== undefined && session.processedLineCount > 0 ) { @@ -4442,7 +4517,7 @@ export class SessionService { events, isCloud: true, logUrl: logUrl ?? session.logUrl, - processedLineCount: totalLineCount, + processedLineCount: effectiveLineCount, }); this.surfacePersistedPendingPermissions(taskRunId, rawEntries); this.pendingPermissionHydratedRuns.add(taskRunId); @@ -4450,12 +4525,52 @@ export class SessionService { // baseline already contains an in-flight session/prompt — the live delta // path otherwise sees delta <= 0 and never re-evaluates the tail. this.updatePromptStateFromEvents(taskRunId, events); - })().catch((err: unknown) => { - this.d.log.warn("Failed to hydrate cloud task session from logs", { - taskId, - taskRunId, - err, + if (isTerminalRun) { + this.clearTerminalCloudPromptState(taskId, taskRunId); + } + })() + .catch((err: unknown) => { + this.d.log.warn("Failed to hydrate cloud task session from logs", { + taskId, + taskRunId, + err, + }); + }) + .finally(() => { + this.hydratingCloudSessions.delete(hydrationKey); }); + } + + private finalizeTerminalCloudTask( + taskId: string, + taskRunId: string, + fields: { + status?: TaskRunStatus; + stage?: string | null; + output?: Record | null; + errorMessage?: string | null; + branch?: string | null; + }, + ): void { + this.d.store.updateCloudStatus(taskRunId, fields); + this.clearTerminalCloudPromptState(taskId, taskRunId); + } + + private clearTerminalCloudPromptState( + taskId: string, + taskRunId: string, + ): void { + const session = this.d.store.getSessions()[taskRunId]; + if ( + !session || + (!session.isPromptPending && session.messageQueue.length === 0) + ) { + return; + } + + this.d.store.clearMessageQueue(taskId); + this.d.store.updateSession(taskRunId, { + isPromptPending: false, }); } @@ -5376,7 +5491,9 @@ export class SessionService { } if (update.kind === "snapshot" && !isTerminalStatus(update.status)) { - this.surfacePersistedPendingPermissions(taskRunId, update.newEntries); + if (!this.isStaleNonTerminalCloudUpdate(taskRunId, update)) { + this.surfacePersistedPendingPermissions(taskRunId, update.newEntries); + } } // NOTE: Don't auto-flush on `!isPromptPending && queue.length > 0` here. @@ -5389,18 +5506,20 @@ export class SessionService { // Update cloud status fields if present if (update.kind === "status" || update.kind === "snapshot") { - this.d.store.updateCloudStatus(taskRunId, { - status: update.status, - stage: update.stage, - output: update.output, - errorMessage: update.errorMessage, - branch: update.branch, - }); - - if (update.status === "in_progress") { - this.tryRecoverIdleCloudQueue(taskRunId, { - serverSandboxAlive: update.sandboxAlive, + if (!this.isStaleNonTerminalCloudUpdate(taskRunId, update)) { + this.d.store.updateCloudStatus(taskRunId, { + status: update.status, + stage: update.stage, + output: update.output, + errorMessage: update.errorMessage, + branch: update.branch, }); + + if (update.status === "in_progress") { + this.tryRecoverIdleCloudQueue(taskRunId, { + serverSandboxAlive: update.sandboxAlive, + }); + } } if (isTerminalStatus(update.status)) { @@ -5422,6 +5541,23 @@ export class SessionService { // --- Helper Methods --- + private isStaleNonTerminalCloudUpdate( + taskRunId: string, + update: CloudTaskUpdatePayload, + ): boolean { + if (update.kind !== "status" && update.kind !== "snapshot") { + return false; + } + if (update.status === undefined) { + return false; + } + const currentCloudStatus = + this.d.store.getSessions()[taskRunId]?.cloudStatus; + return ( + isTerminalStatus(currentCloudStatus) && !isTerminalStatus(update.status) + ); + } + private async resolveCloudPrompt( prompt: string | ContentBlock[], ): Promise { diff --git a/packages/core/src/sessions/sessionStore.ts b/packages/core/src/sessions/sessionStore.ts index 0e86a78c6a..c87339e88a 100644 --- a/packages/core/src/sessions/sessionStore.ts +++ b/packages/core/src/sessions/sessionStore.ts @@ -7,6 +7,7 @@ import type { QueuedMessage, TaskRunStatus, } from "@posthog/shared"; +import { isTerminalStatus } from "@posthog/shared/domain-types"; import { setAutoFreeze } from "immer"; import { immer } from "zustand/middleware/immer"; import { createStore } from "zustand/vanilla"; @@ -130,7 +131,16 @@ export const sessionStoreSetters = { sessionStore.setState((state) => { const session = state.sessions[taskRunId]; if (!session) return; - if (fields.status !== undefined) session.cloudStatus = fields.status; + if (fields.status !== undefined) { + const currentStatus = session.cloudStatus; + if ( + isTerminalStatus(currentStatus) && + !isTerminalStatus(fields.status) + ) { + return; + } + session.cloudStatus = fields.status; + } if (fields.stage !== undefined) session.cloudStage = fields.stage; if (fields.output !== undefined) session.cloudOutput = fields.output; if (fields.errorMessage !== undefined) diff --git a/packages/core/src/sessions/sessionViewState.test.ts b/packages/core/src/sessions/sessionViewState.test.ts new file mode 100644 index 0000000000..1e218fe246 --- /dev/null +++ b/packages/core/src/sessions/sessionViewState.test.ts @@ -0,0 +1,87 @@ +import type { AgentSession } from "@posthog/shared"; +import type { Task, TaskRunStatus } from "@posthog/shared/domain-types"; +import { describe, expect, it } from "vitest"; +import { deriveSessionViewState } from "./sessionViewState"; + +function makeTask(runStatus: TaskRunStatus, runId = "run-1"): Task { + return { + id: "task-1", + task_number: 1, + slug: "task-1", + title: "Task", + description: "", + created_at: "2026-01-01T00:00:00.000Z", + updated_at: "2026-01-01T00:00:00.000Z", + origin_product: "user_created", + latest_run: { + id: runId, + status: runStatus, + environment: "cloud", + } as Task["latest_run"], + }; +} + +function makeSession( + cloudStatus: TaskRunStatus, + taskRunId = "run-1", +): AgentSession { + return { + taskId: "task-1", + taskRunId, + taskTitle: "Task", + channel: `agent-event:${taskRunId}`, + status: "connected", + events: [], + startedAt: 0, + isCloud: true, + cloudStatus, + isPromptPending: false, + isCompacting: false, + promptStartedAt: null, + pendingPermissions: new Map(), + pausedDurationMs: 0, + messageQueue: [], + optimisticItems: [], + }; +} + +describe("deriveSessionViewState", () => { + it("uses terminal task status over stale same-run session status", () => { + const state = deriveSessionViewState( + makeSession("in_progress"), + makeTask("completed"), + null, + true, + ); + + expect(state.cloudStatus).toBe("completed"); + expect(state.isCloudRunTerminal).toBe(true); + expect(state.isInitializing).toBe(false); + }); + + it("uses the task status when the session belongs to an older run", () => { + const state = deriveSessionViewState( + makeSession("completed", "old-run"), + makeTask("in_progress", "new-run"), + null, + true, + ); + + expect(state.cloudStatus).toBe("in_progress"); + expect(state.isCloudRunNotTerminal).toBe(true); + }); + + it("treats not_started as a non-terminal cloud state", () => { + const state = deriveSessionViewState( + undefined, + makeTask("not_started"), + null, + true, + ); + + expect(state.cloudStatus).toBe("not_started"); + expect(state.isCloudRunNotTerminal).toBe(true); + expect(state.isCloudRunTerminal).toBe(false); + expect(state.isInitializing).toBe(true); + }); +}); diff --git a/packages/core/src/sessions/sessionViewState.ts b/packages/core/src/sessions/sessionViewState.ts index de25c07556..cb49afd0a8 100644 --- a/packages/core/src/sessions/sessionViewState.ts +++ b/packages/core/src/sessions/sessionViewState.ts @@ -1,5 +1,9 @@ import type { AcpMessage, AgentSession, Workspace } from "@posthog/shared"; -import type { Task, TaskRunStatus } from "@posthog/shared/domain-types"; +import { + isTerminalStatus, + type Task, + type TaskRunStatus, +} from "@posthog/shared/domain-types"; export interface SessionViewState { isCloudRunNotTerminal: boolean; @@ -23,11 +27,17 @@ export function deriveSessionViewState( workspace: Workspace | null, isCloud: boolean, ): SessionViewState { - const cloudStatus = session?.cloudStatus ?? null; - const isCloudRunNotTerminal = - isCloud && - (!cloudStatus || cloudStatus === "queued" || cloudStatus === "in_progress"); - const isCloudRunTerminal = isCloud && !isCloudRunNotTerminal; + const taskRunId = task.latest_run?.id; + const taskRunStatus = task.latest_run?.status ?? null; + const sessionMatchesLatestRun = + !!taskRunId && session?.taskRunId === taskRunId; + const cloudStatus = sessionMatchesLatestRun + ? isTerminalStatus(taskRunStatus) + ? taskRunStatus + : (session?.cloudStatus ?? taskRunStatus) + : (taskRunStatus ?? session?.cloudStatus ?? null); + const isCloudRunTerminal = isCloud && isTerminalStatus(cloudStatus); + const isCloudRunNotTerminal = isCloud && !isCloudRunTerminal; const hasError = session?.status === "error" && !session?.idleKilled; const handoffInProgress = session?.handoffInProgress ?? false; diff --git a/packages/core/src/task-detail/cloudRunState.test.ts b/packages/core/src/task-detail/cloudRunState.test.ts new file mode 100644 index 0000000000..5f48138e40 --- /dev/null +++ b/packages/core/src/task-detail/cloudRunState.test.ts @@ -0,0 +1,51 @@ +import type { Task, TaskRunStatus } from "@posthog/shared/domain-types"; +import { describe, expect, it } from "vitest"; +import { deriveCloudRunState } from "./cloudRunState"; + +function makeTask(runStatus: TaskRunStatus, runId = "run-1"): Task { + return { + id: "task-1", + task_number: 1, + slug: "task-1", + title: "Task", + description: "", + created_at: "2026-01-01T00:00:00.000Z", + updated_at: "2026-01-01T00:00:00.000Z", + origin_product: "user_created", + latest_run: { + id: runId, + status: runStatus, + environment: "cloud", + } as Task["latest_run"], + }; +} + +describe("deriveCloudRunState", () => { + it("uses terminal task status over stale same-run session status", () => { + const state = deriveCloudRunState( + makeTask("completed"), + { + taskRunId: "run-1", + cloudStatus: "in_progress", + }, + null, + ); + + expect(state.cloudStatus).toBe("completed"); + expect(state.isRunActive).toBe(false); + }); + + it("uses task status when the session belongs to an older run", () => { + const state = deriveCloudRunState( + makeTask("in_progress", "new-run"), + { + taskRunId: "old-run", + cloudStatus: "completed", + }, + null, + ); + + expect(state.cloudStatus).toBe("in_progress"); + expect(state.isRunActive).toBe(true); + }); +}); diff --git a/packages/core/src/task-detail/cloudRunState.ts b/packages/core/src/task-detail/cloudRunState.ts index f076d3ca24..f2989ae34e 100644 --- a/packages/core/src/task-detail/cloudRunState.ts +++ b/packages/core/src/task-detail/cloudRunState.ts @@ -1,6 +1,11 @@ -import type { ChangedFile, Task } from "@posthog/shared/domain-types"; +import { + type ChangedFile, + isTerminalStatus, + type Task, +} from "@posthog/shared/domain-types"; export interface CloudRunSessionLike { + taskRunId?: string | null; cloudBranch?: string | null; cloudStatus?: string | null; } @@ -23,7 +28,15 @@ export function deriveCloudRunState( const effectiveBranch = branch ?? cloudBranch; const repo = task.repository ?? null; - const cloudStatus = session?.cloudStatus ?? task.latest_run?.status ?? null; + const taskRunId = task.latest_run?.id; + const taskRunStatus = task.latest_run?.status ?? null; + const sessionMatchesLatestRun = + !!taskRunId && session?.taskRunId === taskRunId; + const cloudStatus = sessionMatchesLatestRun + ? isTerminalStatus(taskRunStatus) + ? taskRunStatus + : (session?.cloudStatus ?? taskRunStatus) + : (taskRunStatus ?? session?.cloudStatus ?? null); const isRunActive = cloudStatus === "queued" || cloudStatus === "in_progress" || diff --git a/packages/ui/src/features/sessions/sessionServiceHost.test.ts b/packages/ui/src/features/sessions/sessionServiceHost.test.ts index 5d3f9022fb..d2fd777226 100644 --- a/packages/ui/src/features/sessions/sessionServiceHost.test.ts +++ b/packages/ui/src/features/sessions/sessionServiceHost.test.ts @@ -1078,6 +1078,45 @@ describe("SessionService", () => { expect(unsubscribe).not.toHaveBeenCalled(); }); + it("marks a reused same-run watcher terminal when task data reports completion", () => { + const service = getSessionService(); + const unsubscribe = vi.fn(); + const onStatusChange = vi.fn(); + mockTrpcCloudTask.onUpdate.subscribe.mockReturnValueOnce({ + unsubscribe, + }); + + service.watchCloudTask( + "task-123", + "run-123", + "https://api.anthropic.com", + 123, + onStatusChange, + ); + service.watchCloudTask( + "task-123", + "run-123", + "https://api.anthropic.com", + 123, + undefined, + "https://example.com/logs/run-123", + undefined, + "claude", + undefined, + undefined, + undefined, + "completed", + ); + + expect(mockSessionStoreSetters.updateCloudStatus).toHaveBeenCalledWith( + "run-123", + { status: "completed" }, + ); + expect(unsubscribe).toHaveBeenCalledTimes(1); + expect(mockTrpcCloudTask.watch.mutate).toHaveBeenCalledTimes(1); + expect(onStatusChange).not.toHaveBeenCalled(); + }); + it.each<[string, Partial, boolean]>([ [ "skips a hydrated terminal (completed) run", @@ -1133,6 +1172,305 @@ describe("SessionService", () => { ); }); + it("hydrates a caller-reported terminal run without watching it", async () => { + const service = getSessionService(); + const onStatusChange = vi.fn(); + const session = createMockSession({ + taskId: "task-123", + taskRunId: "run-123", + cloudStatus: "in_progress", + events: [ + { + type: "acp_message", + ts: 1700000000, + message: { + jsonrpc: "2.0", + method: "session/update", + params: {}, + }, + } as AcpMessage, + ], + isPromptPending: true, + messageQueue: [ + { + id: "queued-1", + content: "follow up", + queuedAt: 1700000001, + }, + ], + processedLineCount: 1, + }); + mockSessionStoreSetters.getSessionByTaskId.mockReturnValue(session); + mockSessionStoreSetters.getSessions.mockReturnValue({ + "run-123": session, + }); + const finalEntries = [ + { timestamp: "2024-01-01T00:00:00Z", notification: {} }, + { timestamp: "2024-01-01T00:01:00Z", notification: {} }, + ]; + const finalEvents = [ + { + type: "acp_message", + ts: 1700000000, + message: { + jsonrpc: "2.0", + method: "session/update", + params: {}, + }, + } as AcpMessage, + { + type: "acp_message", + ts: 1700000001, + message: { + jsonrpc: "2.0", + method: "session/update", + params: {}, + }, + } as AcpMessage, + ]; + mockAuthenticatedClient.getTaskRunSessionLogs.mockResolvedValue( + finalEntries, + ); + mockConvertStoredEntriesToEvents.mockReturnValueOnce(finalEvents); + + service.watchCloudTask( + "task-123", + "run-123", + "https://api.anthropic.com", + 123, + onStatusChange, + "https://example.com/logs/run-123", + undefined, + "claude", + undefined, + undefined, + undefined, + "completed", + ); + + expect(mockSessionStoreSetters.updateCloudStatus).toHaveBeenCalledWith( + "run-123", + { status: "completed" }, + ); + expect(mockSessionStoreSetters.clearMessageQueue).toHaveBeenCalledWith( + "task-123", + ); + expect(mockSessionStoreSetters.updateSession).toHaveBeenCalledWith( + "run-123", + expect.objectContaining({ isPromptPending: false }), + ); + + expect(mockTrpcCloudTask.onUpdate.subscribe).not.toHaveBeenCalled(); + expect(mockTrpcCloudTask.watch.mutate).not.toHaveBeenCalled(); + expect(onStatusChange).not.toHaveBeenCalled(); + await vi.waitFor(() => { + expect( + mockAuthenticatedClient.getTaskRunSessionLogs, + ).toHaveBeenCalledWith("task-123", "run-123", { limit: 100000 }); + }); + expect(mockSessionStoreSetters.updateSession).toHaveBeenCalledWith( + "run-123", + expect.objectContaining({ + events: finalEvents, + processedLineCount: finalEntries.length, + }), + ); + }); + + it("falls back to the run log URL when terminal chain hydration is empty", async () => { + const service = getSessionService(); + const session = createMockSession({ + taskId: "task-123", + taskRunId: "run-123", + cloudStatus: "in_progress", + isCloud: true, + events: [], + }); + mockSessionStoreSetters.getSessionByTaskId.mockReturnValue(session); + mockSessionStoreSetters.getSessions.mockReturnValue({ + "run-123": session, + }); + mockAuthenticatedClient.getTaskRunSessionLogs.mockResolvedValue([]); + mockTrpcLogs.readLocalLogs.query.mockResolvedValue(""); + mockTrpcLogs.fetchS3Logs.query.mockResolvedValue( + JSON.stringify({ + type: "notification", + timestamp: "2024-01-01T00:00:00Z", + notification: { + method: "session/update", + params: {}, + }, + }), + ); + mockTrpcLogs.writeLocalLogs.mutate.mockResolvedValue(undefined); + const finalEvents = [ + { + type: "acp_message", + ts: 1700000000, + message: { + jsonrpc: "2.0", + method: "session/update", + params: {}, + }, + } as AcpMessage, + ]; + mockConvertStoredEntriesToEvents.mockReturnValueOnce(finalEvents); + + service.watchCloudTask( + "task-123", + "run-123", + "https://api.anthropic.com", + 123, + undefined, + "https://example.com/logs/run-123", + undefined, + "claude", + undefined, + "build me a thing", + undefined, + "completed", + ); + + await vi.waitFor(() => { + expect(mockTrpcLogs.fetchS3Logs.query).toHaveBeenCalledWith({ + logUrl: "https://example.com/logs/run-123", + }); + }); + await vi.waitFor(() => { + expect(mockSessionStoreSetters.updateSession).toHaveBeenCalledWith( + "run-123", + expect.objectContaining({ + events: finalEvents, + processedLineCount: 1, + }), + ); + }); + expect( + mockSessionStoreSetters.clearTailOptimisticItems, + ).toHaveBeenCalledWith("run-123"); + expect( + mockSessionStoreSetters.appendOptimisticItem, + ).not.toHaveBeenCalled(); + }); + + it("does not cache or seed an empty terminal hydration", async () => { + const service = getSessionService(); + const session = createMockSession({ + taskId: "task-123", + taskRunId: "run-123", + cloudStatus: "in_progress", + isCloud: true, + events: [], + }); + mockSessionStoreSetters.getSessionByTaskId.mockReturnValue(session); + mockSessionStoreSetters.getSessions.mockReturnValue({ + "run-123": session, + }); + mockAuthenticatedClient.getTaskRunSessionLogs.mockResolvedValue([]); + mockTrpcLogs.readLocalLogs.query.mockResolvedValue(""); + mockTrpcLogs.fetchS3Logs.query.mockResolvedValue(""); + + service.watchCloudTask( + "task-123", + "run-123", + "https://api.anthropic.com", + 123, + undefined, + "https://example.com/logs/run-123", + undefined, + "claude", + undefined, + "build me a thing", + undefined, + "completed", + ); + + await vi.waitFor(() => { + expect( + mockSessionStoreSetters.clearTailOptimisticItems, + ).toHaveBeenCalledWith("run-123"); + }); + expect( + mockSessionStoreSetters.appendOptimisticItem, + ).not.toHaveBeenCalled(); + expect(mockSessionStoreSetters.updateSession).not.toHaveBeenCalledWith( + "run-123", + expect.objectContaining({ processedLineCount: 0 }), + ); + }); + + it("starts terminal hydration even when resume-chain hydration is already in flight", async () => { + const service = getSessionService(); + const session = createMockSession({ + taskId: "task-123", + taskRunId: "run-123", + isCloud: true, + events: [], + }); + mockSessionStoreSetters.getSessionByTaskId.mockReturnValue(session); + mockSessionStoreSetters.getSessions.mockReturnValue({ + "run-123": session, + }); + let resolveFirstHydration!: ( + entries: Array<{ timestamp: string; notification: object }>, + ) => void; + const firstHydration = new Promise< + Array<{ timestamp: string; notification: object }> + >((resolve) => { + resolveFirstHydration = resolve; + }); + mockAuthenticatedClient.getTaskRunSessionLogs + .mockReturnValueOnce(firstHydration) + .mockResolvedValueOnce([]); + + service.watchCloudTask( + "task-123", + "run-123", + "https://api.anthropic.com", + 123, + undefined, + "https://example.com/logs/run-123", + undefined, + "claude", + undefined, + undefined, + undefined, + "in_progress", + undefined, + { resume_from_run_id: "previous-run" }, + ); + + await vi.waitFor(() => { + expect( + mockAuthenticatedClient.getTaskRunSessionLogs, + ).toHaveBeenCalledTimes(1); + }); + + service.watchCloudTask( + "task-123", + "run-123", + "https://api.anthropic.com", + 123, + undefined, + "https://example.com/logs/run-123", + undefined, + "claude", + undefined, + undefined, + undefined, + "completed", + undefined, + { resume_from_run_id: "previous-run" }, + ); + + await vi.waitFor(() => { + expect( + mockAuthenticatedClient.getTaskRunSessionLogs, + ).toHaveBeenCalledTimes(2); + }); + resolveFirstHydration([]); + }); + it("does not re-subscribe across repeated calls for a hydrated terminal run", () => { const service = getSessionService(); mockSessionStoreSetters.getSessionByTaskId.mockReturnValue( @@ -1198,6 +1536,47 @@ describe("SessionService", () => { expect(onStatusChange).toHaveBeenCalledTimes(1); }); + it("ignores stale non-terminal stream status after a terminal status", () => { + const service = getSessionService(); + const onStatusChange = vi.fn(); + const completedSession = createMockSession({ + taskId: "task-123", + taskRunId: "run-123", + isCloud: true, + cloudStatus: "completed", + }); + mockSessionStoreSetters.getSessions.mockReturnValue({ + "run-123": completedSession, + }); + + service.watchCloudTask( + "task-123", + "run-123", + "https://api.anthropic.com", + 123, + onStatusChange, + ); + + const subscribeOptions = mockTrpcCloudTask.onUpdate.subscribe.mock + .calls[0][1] as { + onData: (update: { + kind: "status"; + taskId: string; + runId: string; + status: "in_progress"; + }) => void; + }; + subscribeOptions.onData({ + kind: "status", + taskId: "task-123", + runId: "run-123", + status: "in_progress", + }); + + expect(mockSessionStoreSetters.updateCloudStatus).not.toHaveBeenCalled(); + expect(onStatusChange).not.toHaveBeenCalled(); + }); + it("hydrates a fresh cloud session from persisted logs before replay arrives", async () => { const service = getSessionService(); const hydratedSession = createMockSession({ diff --git a/packages/ui/src/features/sessions/sessionStore.test.ts b/packages/ui/src/features/sessions/sessionStore.test.ts index 25d58fb5ba..d088a7ed31 100644 --- a/packages/ui/src/features/sessions/sessionStore.test.ts +++ b/packages/ui/src/features/sessions/sessionStore.test.ts @@ -116,6 +116,47 @@ describe("dequeueMessages", () => { }); }); +describe("updateCloudStatus", () => { + beforeEach(() => { + useSessionStore.setState((state) => { + state.sessions = {}; + state.taskIdIndex = {}; + }); + }); + + it("does not downgrade a terminal run when a stale non-terminal status arrives", () => { + sessionStoreSetters.setSession({ + taskRunId: "run-123", + taskId: "task-123", + taskTitle: "Test", + channel: "agent-event:run-123", + events: [], + startedAt: 0, + status: "connected", + isPromptPending: false, + isCompacting: false, + promptStartedAt: null, + pendingPermissions: new Map(), + pausedDurationMs: 0, + messageQueue: [], + optimisticItems: [], + cloudStatus: "completed", + }); + + sessionStoreSetters.updateCloudStatus("run-123", { + status: "in_progress", + branch: "stale-branch", + }); + + expect(useSessionStore.getState().sessions["run-123"].cloudStatus).toBe( + "completed", + ); + expect(useSessionStore.getState().sessions["run-123"].cloudBranch).toBe( + undefined, + ); + }); +}); + describe("dequeueMessagesAsText", () => { beforeEach(() => { useSessionStore.setState((state) => { diff --git a/packages/ui/src/features/task-detail/hooks/useCloudRunState.ts b/packages/ui/src/features/task-detail/hooks/useCloudRunState.ts index 88d2d82518..2bc25a5402 100644 --- a/packages/ui/src/features/task-detail/hooks/useCloudRunState.ts +++ b/packages/ui/src/features/task-detail/hooks/useCloudRunState.ts @@ -4,14 +4,19 @@ import type { Task } from "@posthog/shared/domain-types"; import { useMemo } from "react"; import { resolveCloudPrUrl } from "../../git-interaction/cloudPrUrl"; import { useSessionForTask } from "../../sessions/useSession"; +import { pickFreshestTask } from "../../tasks/taskFreshness"; import { useTasks } from "../../tasks/useTasks"; import { useCloudEventSummary } from "./useCloudEventSummary"; export function useCloudRunState(taskId: string, task: Task) { const { data: tasks = [] } = useTasks(); const freshTask = useMemo( - () => tasks.find((t) => t.id === taskId) ?? task, - [tasks, taskId, task], + () => + pickFreshestTask( + task, + tasks.find((t) => t.id === taskId), + ) ?? task, + [task, taskId, tasks], ); const session = useSessionForTask(taskId); diff --git a/packages/ui/src/features/task-detail/hooks/useTaskData.ts b/packages/ui/src/features/task-detail/hooks/useTaskData.ts index ae278ba306..3818aa360e 100644 --- a/packages/ui/src/features/task-detail/hooks/useTaskData.ts +++ b/packages/ui/src/features/task-detail/hooks/useTaskData.ts @@ -9,6 +9,7 @@ import { useWorkspaceTRPC } from "@posthog/workspace-client/trpc"; import { useQuery } from "@tanstack/react-query"; import { useMemo } from "react"; import { cloneStore } from "../../clone/cloneStore"; +import { pickFreshestTask } from "../../tasks/taskFreshness"; import { useTasks } from "../../tasks/useTasks"; import { useWorkspace } from "../../workspace/useWorkspace"; @@ -22,8 +23,12 @@ export function useTaskData({ taskId, initialTask }: UseTaskDataParams) { const { data: tasks = [] } = useTasks(); const task = useMemo( - () => tasks.find((t) => t.id === taskId) || initialTask, - [tasks, taskId, initialTask], + () => + pickFreshestTask( + initialTask, + tasks.find((t) => t.id === taskId), + ) ?? initialTask, + [initialTask, taskId, tasks], ); const workspace = useWorkspace(taskId); diff --git a/packages/ui/src/features/tasks/queries.test.ts b/packages/ui/src/features/tasks/queries.test.ts new file mode 100644 index 0000000000..d18e7e6206 --- /dev/null +++ b/packages/ui/src/features/tasks/queries.test.ts @@ -0,0 +1,18 @@ +import { describe, expect, it } from "vitest"; +import { isTaskDetailNotFoundError } from "./queries"; + +describe("task queries", () => { + it("detects task detail 404 errors from the shared API fetcher", () => { + expect( + isTaskDetailNotFoundError( + new Error('Failed request: [404] {"detail":"Not found."}'), + ), + ).toBe(true); + expect( + isTaskDetailNotFoundError( + new Error('Failed request: [500] {"detail":"Server error."}'), + ), + ).toBe(false); + expect(isTaskDetailNotFoundError("Failed request: [404]")).toBe(false); + }); +}); diff --git a/packages/ui/src/features/tasks/queries.ts b/packages/ui/src/features/tasks/queries.ts index 5333809331..3d70a850c5 100644 --- a/packages/ui/src/features/tasks/queries.ts +++ b/packages/ui/src/features/tasks/queries.ts @@ -26,6 +26,10 @@ export function taskDetailQuery(taskId: string) { }); } +export function isTaskDetailNotFoundError(error: unknown): boolean { + return error instanceof Error && error.message.includes("[404]"); +} + // Read a task from the already-loaded sidebar list cache without fetching. // Lets the task-detail route loader resolve synchronously from cache. export function getCachedTask(taskId: string): Task | undefined { diff --git a/packages/ui/src/features/tasks/taskFreshness.test.ts b/packages/ui/src/features/tasks/taskFreshness.test.ts new file mode 100644 index 0000000000..71d6a8815f --- /dev/null +++ b/packages/ui/src/features/tasks/taskFreshness.test.ts @@ -0,0 +1,65 @@ +import type { Task } from "@posthog/shared/domain-types"; +import { describe, expect, it } from "vitest"; +import { pickFreshestTask } from "./taskFreshness"; + +function makeTask( + title: string, + updatedAt: string, + runUpdatedAt?: string, +): Task { + return { + id: title, + task_number: 1, + slug: title, + title, + description: "", + created_at: "2026-01-01T00:00:00.000Z", + updated_at: updatedAt, + origin_product: "user_created", + latest_run: runUpdatedAt + ? ({ + id: `${title}-run`, + updated_at: runUpdatedAt, + } as Task["latest_run"]) + : undefined, + }; +} + +describe("pickFreshestTask", () => { + it("prefers the first task when timestamps tie", () => { + const detail = makeTask("detail", "2026-01-01T00:00:00.000Z"); + const list = makeTask("list", "2026-01-01T00:00:00.000Z"); + + expect(pickFreshestTask(detail, list)).toBe(detail); + }); + + it("uses latest run activity when it is newer than the task", () => { + const completedDetail = makeTask( + "detail", + "2026-01-01T00:00:00.000Z", + "2026-01-01T00:05:00.000Z", + ); + const staleList = makeTask( + "list", + "2026-01-01T00:03:00.000Z", + "2026-01-01T00:03:00.000Z", + ); + + expect(pickFreshestTask(staleList, completedDetail)).toBe(completedDetail); + }); + + it("keeps a newer list update when it arrives after detail data", () => { + const detail = makeTask( + "detail", + "2026-01-01T00:00:00.000Z", + "2026-01-01T00:05:00.000Z", + ); + const newerList = makeTask( + "list", + "2026-01-01T00:06:00.000Z", + "2026-01-01T00:06:00.000Z", + ); + + expect(pickFreshestTask(detail, newerList)).toBe(newerList); + }); +}); diff --git a/packages/ui/src/features/tasks/taskFreshness.ts b/packages/ui/src/features/tasks/taskFreshness.ts new file mode 100644 index 0000000000..19339e78fe --- /dev/null +++ b/packages/ui/src/features/tasks/taskFreshness.ts @@ -0,0 +1,32 @@ +import type { Task } from "@posthog/shared/domain-types"; + +function parseTime(value: string | null | undefined): number { + const timestamp = value ? Date.parse(value) : Number.NEGATIVE_INFINITY; + return Number.isFinite(timestamp) ? timestamp : Number.NEGATIVE_INFINITY; +} + +export function getTaskFreshness(task: Task): number { + return Math.max( + parseTime(task.updated_at), + parseTime(task.latest_run?.updated_at), + ); +} + +export function pickFreshestTask( + ...tasks: Array +): Task | undefined { + let selected: Task | undefined; + let selectedFreshness = Number.NEGATIVE_INFINITY; + + for (const task of tasks) { + if (!task) continue; + + const freshness = getTaskFreshness(task); + if (!selected || freshness > selectedFreshness) { + selected = task; + selectedFreshness = freshness; + } + } + + return selected; +} diff --git a/packages/ui/src/router/routes/code/tasks/$taskId.tsx b/packages/ui/src/router/routes/code/tasks/$taskId.tsx index a4e2145fde..898483217e 100644 --- a/packages/ui/src/router/routes/code/tasks/$taskId.tsx +++ b/packages/ui/src/router/routes/code/tasks/$taskId.tsx @@ -3,8 +3,10 @@ import { TaskDetail } from "@posthog/ui/features/task-detail/components/TaskDeta import { getCachedTask, getCachedTaskDetail, + isTaskDetailNotFoundError, taskDetailQuery, } from "@posthog/ui/features/tasks/queries"; +import { pickFreshestTask } from "@posthog/ui/features/tasks/taskFreshness"; import { useTasks } from "@posthog/ui/features/tasks/useTasks"; import { TaskDetailSkeleton } from "@posthog/ui/router/routeSkeletons"; import { yieldToPaint } from "@posthog/ui/router/yieldToPaint"; @@ -37,26 +39,27 @@ function TaskDetailRoute() { const loaderTask = Route.useLoaderData(); const { data: tasks } = useTasks(); const fromList = tasks?.find((t) => t.id === taskId); + const initialTask = pickFreshestTask(fromList, loaderTask); // Cold deep-link / URL restore: nothing cached. Fetch the single task here so // a hang or 404 only affects this view's spinner, never the router. - const needsFetch = !fromList && !loaderTask; + const needsFetch = !initialTask; const { data: fetched, + error, isError, isSuccess, - } = useQuery({ - ...taskDetailQuery(taskId), - enabled: needsFetch, - }); + } = useQuery(taskDetailQuery(taskId)); - // Prefer the live list task (kept fresh by polling + subscriptions). - const task = fromList ?? loaderTask ?? fetched; + const task = pickFreshestTask(fetched, initialTask); // Task doesn't exist (deleted, 404, or stale deep link): the cold fetch // settled with an error or empty result. Redirect to the new-task screen // rather than spin forever — matches the old navigationStore.hydrateTask. - if (needsFetch && (isError || (isSuccess && !fetched))) { + if ( + isTaskDetailNotFoundError(error) || + (needsFetch && (isError || (isSuccess && !fetched))) + ) { return ; } diff --git a/packages/ui/src/router/routes/website/$channelId/tasks/$taskId.tsx b/packages/ui/src/router/routes/website/$channelId/tasks/$taskId.tsx index 85fbec93f8..08a1281509 100644 --- a/packages/ui/src/router/routes/website/$channelId/tasks/$taskId.tsx +++ b/packages/ui/src/router/routes/website/$channelId/tasks/$taskId.tsx @@ -7,13 +7,16 @@ import { TaskDetail } from "@posthog/ui/features/task-detail/components/TaskDeta import { getCachedTask, getCachedTaskDetail, + isTaskDetailNotFoundError, taskDetailQuery, } from "@posthog/ui/features/tasks/queries"; +import { pickFreshestTask } from "@posthog/ui/features/tasks/taskFreshness"; import { useTasks } from "@posthog/ui/features/tasks/useTasks"; import { TaskDetailSkeleton } from "@posthog/ui/router/routeSkeletons"; import { yieldToPaint } from "@posthog/ui/router/yieldToPaint"; +import { Button, Flex, Text } from "@radix-ui/themes"; import { useQuery } from "@tanstack/react-query"; -import { createFileRoute } from "@tanstack/react-router"; +import { createFileRoute, Navigate } from "@tanstack/react-router"; import { useEffect } from "react"; export const Route = createFileRoute("/website/$channelId/tasks/$taskId")({ @@ -37,6 +40,7 @@ function ChannelTaskDetailRoute() { const loaderTask = Route.useLoaderData(); const { data: tasks } = useTasks(); const fromList = tasks?.find((t) => t.id === taskId); + const initialTask = pickFreshestTask(fromList, loaderTask); const { channels } = useChannels(); const channelName = channels.find((c) => c.id === channelId)?.name; @@ -56,12 +60,49 @@ function ChannelTaskDetailRoute() { openThread(taskId, { expand: false }); }, [openThread, taskId]); - const { data: fetched } = useQuery({ + const { + data: fetched, + error, + isError, + isFetching, + isSuccess, + refetch, + } = useQuery({ ...taskDetailQuery(taskId), - enabled: !fromList && !loaderTask, }); - const task = fromList ?? loaderTask ?? fetched; + const task = pickFreshestTask(fetched, initialTask); + + if (isTaskDetailNotFoundError(error)) { + return ; + } + + if (!task && isSuccess && !fetched) { + return ; + } + + if (!task && isError) { + const message = + error instanceof Error ? error.message : "Failed to load task"; + return ( + + + Failed to load task + + {message} + + + + + ); + } if (!task) { return ; From 4b92e4efc7ca57860349c952c0cbdcc0a8b77a00 Mon Sep 17 00:00:00 2001 From: Alessandro Pogliaghi Date: Tue, 7 Jul 2026 15:34:33 +0100 Subject: [PATCH 08/11] fix(agent): export error provenance only and isolate telemetry flush Addresses four review findings on the run telemetry export: - _posthog/error records and the root span no longer carry the raw error string: it is free text that can embed prompt or repo content (exception paths, provider errors). Exported signal is now the generic "run error" body plus error_source/stop_reason attributes; the full message stays in the session log and the task run's error_message. - Telemetry flush/shutdown use Promise.allSettled so logs and traces complete independently, and both batch processors cap exports at 5s (exportTimeoutMillis) instead of the SDK's 30s default - a rejecting or hanging traces endpoint can no longer starve log delivery or delay session cleanup. - A run error now closes still-open tool spans as ERROR with tool_status=interrupted, so APM never shows a healthy-looking active tool under a failed run. - OtelTransportConfig/AgentConfig.otelTransport are restored as @deprecated ignored stubs: @posthog/agent is published, so removing exported types is an API break reserved for a major. Generated-By: PostHog Code Task-Id: 0c511836-2180-455a-9b58-45df2a0661ec --- REPORT.md | 13 ++-- packages/agent/README.md | 4 +- packages/agent/src/otel-attributes.ts | 5 +- packages/agent/src/otel-telemetry.test.ts | 83 ++++++++++++++++++++--- packages/agent/src/otel-telemetry.ts | 29 ++++++-- packages/agent/src/otel-trace-builder.ts | 36 +++++++--- packages/agent/src/types.ts | 17 +++++ 7 files changed, 149 insertions(+), 38 deletions(-) diff --git a/REPORT.md b/REPORT.md index 0d414c32d8..08c0cc32bc 100644 --- a/REPORT.md +++ b/REPORT.md @@ -54,7 +54,7 @@ Exported events (allowlist, everything else is dropped): | `_posthog/usage_update` (and the `session/update` variant) | info | `tokens_input/output/cached_read/cached_write`, `cost_usd` | | `_posthog/turn_complete` | info | `stop_reason` | | `_posthog/task_complete` | info | `stop_reason` | -| `_posthog/error` | **error** | `error_source`, `stop_reason`, message in body (capped) | +| `_posthog/error` | **error** | `error_source`, `stop_reason`; body is the generic "run error" — the raw message is free text that can embed prompt/repo content, so it stays in the session log and the run's `error_message` | | `_posthog/progress` | info | `progress_group/step/status` | | `_posthog/git_checkpoint`, `_posthog/branch_created` | info | `branch` | | `_posthog/mode_change`, `_posthog/compact_boundary` | info | | @@ -67,10 +67,10 @@ Bodies are capped at 2000 chars; free-text attribute values at 200 chars (the `l ### APM trace (one per run) -- `task_run` root span (kind SERVER): opened at session init, closed at session cleanup. Status is resolved at shutdown from the latest turn outcome (the sandbox never emits `task_complete` for successful runs — the terminal "completed" status is decided by the workflow outside — so the last turn is the in-sandbox success signal): OK when the last turn ended with `end_turn`, ERROR with the error message on `_posthog/error` (an error always wins, later turn completions cannot flip it back) or when the last turn stopped with `error`, unset otherwise (cancelled / refused / timed out / no completed turns). Resolving at shutdown rather than per-turn means an early clean turn cannot leave a stale OK on a run whose final turn was cancelled. +- `task_run` root span (kind SERVER): opened at session init, closed at session cleanup. Status is resolved at shutdown from the latest turn outcome (the sandbox never emits `task_complete` for successful runs — the terminal "completed" status is decided by the workflow outside — so the last turn is the in-sandbox success signal): OK when the last turn ended with `end_turn`, ERROR on `_posthog/error` (an error always wins, later turn completions cannot flip it back; `error_source` lands as a root-span attribute while the raw message is withheld — see the error row above) or when the last turn stopped with `error`, unset otherwise (cancelled / refused / timed out / no completed turns). Resolving at shutdown rather than per-turn means an early clean turn cannot leave a stale OK on a run whose final turn was cancelled. - `turn` child spans: opened on each ACP `session/prompt` (used purely as a boundary marker; its content is never read), closed on `_posthog/turn_complete`; attributes `turn_index`, `stop_reason`, plus the turn's token counts and `cost_usd` lifted from usage updates, so APM can rank slow or expensive turns directly. - `tool_call:` grandchild spans (`execute`, `read`, `edit`, ...): opened on `tool_call`, closed on the terminal `tool_call_update`; status ERROR on `failed`; attributes `tool_call_id`, `tool_kind`, `tool_status`. Per-kind span names stay low-cardinality and make APM latency breakdowns by tool kind useful. -- Robustness: orphaned spans are closed (status unset) and exported at shutdown; a new prompt while a turn is open closes the stale turn; duplicate `tool_call` events are idempotent; an error cascades ERROR status through open tool and turn spans to the root. +- Robustness: orphaned spans are closed (status unset) and exported at shutdown; a new prompt while a turn is open closes the stale turn; duplicate `tool_call` events are idempotent; a run error cascades ERROR status to the open turn and the root, and closes still-open tool spans as ERROR with `tool_status=interrupted` so APM never shows a healthy-looking active tool under a failed run. - Every log record is emitted under the OTel context of the span it belongs to (tool logs on the tool span, lifecycle logs on the root), so `trace_id`/`span_id` land in the `logs` table columns and the UI links Logs ⇄ trace waterfall. ### Delivery timing @@ -78,6 +78,7 @@ Bodies are capped at 2000 chars; free-text attribute values at 200 chars (the `l Telemetry is near-realtime, not end-of-turn: records are created the moment each notification flows through the writer and batched for at most 2 s (`BatchLogRecordProcessor` / `BatchSpanProcessor`, `scheduledDelayMillis` 2000). Spans export when they end (tools mid-turn, turns at `turn_complete`, root at cleanup). Flush safety nets: an explicit flush after a terminal error in `signalTaskComplete`, a full shutdown-flush in `cleanupSession` (which SIGTERM reaches via `stop()`), so sandbox teardown cannot eat the tail of a run's telemetry. +Flush and shutdown are best-effort and per-signal independent (`Promise.allSettled`), and every export is capped at 5 s (`exportTimeoutMillis`, down from the SDK's 30 s default), so a rejecting or hanging traces endpoint can neither starve log delivery nor hold up session cleanup. ## Changes in PostHog/code (this branch) @@ -87,9 +88,9 @@ Flush safety nets: an explicit flush after a terminal error in `signalTaskComple - `packages/agent/src/session-log-writer.ts`: optional `sinks: SessionLogSink[]`, teed in `appendRawLine` after the entry is built; a throwing sink warns once and can never break product log persistence; message chunks never reach sinks. `SessionContext` moved here from the otel module. - `packages/agent/src/server/agent-server.ts`: builds the telemetry per session from config (`createRunTelemetry`), passes it as the writer sink, stores it on the session, shuts it down in `cleanupSession`, flushes after terminal errors, and mirrors `enqueueTaskTerminalEvent` payloads into it directly (terminal `_posthog/error` events bypass `SessionLogWriter`, and a failed run is exactly what telemetry must record). Fatal crashes (`reportFatalError`, the uncaught-exception/unhandled-rejection path) also mirror an error record (`error_source=agent_server_crash`) and shut telemetry down, so hard process deaths reach the telemetry project instead of vanishing. - `packages/agent/src/server/bin.ts` + `server/types.ts`: zod-validated env `POSTHOG_AGENT_OTEL_LOGS_URL`, `POSTHOG_AGENT_OTEL_LOGS_TOKEN`, `POSTHOG_AGENT_OTEL_TRACES_URL` → `AgentServerConfig.otelLogsUrl/otelLogsToken/otelTracesUrl`. Telemetry is off unless the logs pair is set; spans additionally require the traces URL (per-signal kill switch). -- `packages/agent/src/types.ts`: deleted the dead `OtelTransportConfig`/`AgentConfig.otelTransport` left over from the February attempt. +- `packages/agent/src/types.ts`: the February `OtelTransportConfig`/`AgentConfig.otelTransport` remain as `@deprecated`, ignored stubs — `@posthog/agent` is a published package, so removing exported types is an API break reserved for a major. - Dependencies: `@opentelemetry/api`, `@opentelemetry/sdk-trace-base`, `@opentelemetry/exporter-trace-otlp-http`, version-aligned with the existing logs SDK (0.208.x experimental / 2.x stable line). -- Tests (71 passing): parameterized log-mapping matrix, a hard privacy test asserting exported payloads never contain tool args/titles/output, per-user resource attributes, session-mismatch guard, never-throws guard, sink isolation in `SessionLogWriter`, and four trace tests (span tree + statuses + attributes, log⇄span id correlation, error cascade, orphan export on shutdown). +- Tests (74 passing): parameterized log-mapping matrix, hard privacy tests asserting exported payloads never contain tool args/titles/output or raw error messages, per-user resource attributes, session-mismatch guard, never-throws guard, sink isolation in `SessionLogWriter`, and five trace tests (span tree + statuses + attributes, log⇄span id correlation, error cascade incl. interrupted tools, orphan export on shutdown, log shutdown isolated from a failing traces endpoint). - `packages/agent/README.md`: documents the env vars and behavior. ## Changes in PostHog/posthog (companion branch) @@ -117,7 +118,7 @@ Flush safety nets: an explicit flush after a terminal error in `signalTaskComple ## Verification -- `PostHog/code`: 71 tests pass in the agent package (including the new telemetry suite), `tsc --noEmit` clean via turbo, biome clean on all touched files (one pre-existing warning untouched). The package's pre-existing test failures in this environment (missing Postgres/git fixtures) were confirmed byte-identical with and without these changes by running the failing files against a stashed tree. +- `PostHog/code`: 74 tests pass in the agent package (including the new telemetry suite), `tsc --noEmit` clean via turbo, biome clean on all touched files (one pre-existing warning untouched). The package's pre-existing test failures in this environment (missing Postgres/git fixtures) were confirmed byte-identical with and without these changes by running the failing files against a stashed tree. - `PostHog/posthog`: 21 tests pass across `test_provision_sandbox.py` and the new `TestBuildSandboxEnvironmentVariables`; `ruff check`/`format` clean on all touched files. DB-dependent suites in this sandbox fail identically with and without the change (no Postgres available). - Local-dev routing verified: Caddy serves `/i/v1/logs`/`/i/v1/traces` on `localhost:8000` and proxies to `capture-logs`, and the Docker URL rewrite covers the new vars. diff --git a/packages/agent/README.md b/packages/agent/README.md index 4943731edd..985946a92e 100644 --- a/packages/agent/README.md +++ b/packages/agent/README.md @@ -172,7 +172,7 @@ Optional run telemetry (the logs pair must both be set, otherwise telemetry stay - `POSTHOG_AGENT_OTEL_LOGS_TOKEN` — project API key of the telemetry project - `POSTHOG_AGENT_OTEL_TRACES_URL` — full OTLP traces URL, e.g. `https://us.i.posthog.com/i/v1/traces`; additionally enables one APM trace per run -When set, `AgentServer` ships an allowlisted metadata subset of the session log (run/turn/tool lifecycle, usage, errors — never message content or tool arguments; see `src/otel-telemetry.ts`) to PostHog Logs, tagged with `service.name=posthog-code-agent` and `run_id`/`task_id`/`team_id`/`user_id`/`distinct_id` resource attributes so cloud runs are filterable per user. With the traces URL set, each run also produces an APM trace (`task_run` root span, a `turn` span per prompt, a `tool_call:` span per tool call; see `src/otel-trace-builder.ts`), and log records carry the matching trace/span ids so Logs and APM cross-link. +When set, `AgentServer` ships an allowlisted metadata subset of the session log (run/turn/tool lifecycle, usage, error provenance — never message content, tool arguments, or raw error text; see `src/otel-telemetry.ts`) to PostHog Logs, tagged with `service.name=posthog-code-agent` and `run_id`/`task_id`/`team_id`/`user_id`/`distinct_id` resource attributes so cloud runs are filterable per user. With the traces URL set, each run also produces an APM trace (`task_run` root span, a `turn` span per prompt, a `tool_call:` span per tool call; see `src/otel-trace-builder.ts`), and log records carry the matching trace/span ids so Logs and APM cross-link. ## Agent SDK @@ -214,7 +214,7 @@ Logs serve two purposes: real-time observability and session resume. Every ACP m `SessionLogWriter` (`src/session-log-writer.ts`) is a per-session multiplexer that buffers raw ndJson lines. On flush (auto-scheduled 500ms after writes, or explicit), it POSTs batched `StoredNotification` entries to the Django API via `PostHogAPIClient.appendTaskRunLog()`; the API stores them in S3 as the task run's `log_url`. This S3 log is the source of truth for the full transcript and for session resume. -Independently of that flush cycle, the writer tees every parsed non-chunk entry to optional `SessionLogSink`s at append time. In cloud, `OtelRunTelemetry` (`src/otel-telemetry.ts`) is wired as a sink and exports an allowlisted metadata subset (run/turn/tool lifecycle, usage, errors — never message content or tool arguments) to PostHog Logs, plus an APM trace per run when configured. See "Optional run telemetry" above for the env vars and behavior. Sink failures can never break S3 log persistence. +Independently of that flush cycle, the writer tees every parsed non-chunk entry to optional `SessionLogSink`s at append time. In cloud, `OtelRunTelemetry` (`src/otel-telemetry.ts`) is wired as a sink and exports an allowlisted metadata subset (run/turn/tool lifecycle, usage, error provenance — never message content, tool arguments, or raw error text) to PostHog Logs, plus an APM trace per run when configured. See "Optional run telemetry" above for the env vars and behavior. Sink failures can never break S3 log persistence. ### Resuming from logs diff --git a/packages/agent/src/otel-attributes.ts b/packages/agent/src/otel-attributes.ts index b2014c5d06..85cd41eee3 100644 --- a/packages/agent/src/otel-attributes.ts +++ b/packages/agent/src/otel-attributes.ts @@ -2,11 +2,14 @@ const MAX_BODY_CHARS = 2000; // PostHog Logs only facets attribute key/value pairs shorter than 256 chars, // so free-text attribute values are capped well below that. const MAX_ATTR_CHARS = 200; +// The SDK default export timeout is 30s; a hanging endpoint must not hold up +// session cleanup (the sandbox can be torn down right after), so keep it short. +const EXPORT_TIMEOUT_MS = 5000; export type AttributeValue = string | number | boolean; export type Attributes = Record; -export { MAX_ATTR_CHARS, MAX_BODY_CHARS }; +export { EXPORT_TIMEOUT_MS, MAX_ATTR_CHARS, MAX_BODY_CHARS }; export function truncate(value: string, max: number): string { return value.length <= max ? value : `${value.slice(0, max)}…`; diff --git a/packages/agent/src/otel-telemetry.test.ts b/packages/agent/src/otel-telemetry.test.ts index 456b40a689..9b761521b7 100644 --- a/packages/agent/src/otel-telemetry.test.ts +++ b/packages/agent/src/otel-telemetry.test.ts @@ -11,17 +11,20 @@ const mockSpanExport = vi.fn((_spans, callback) => { callback({ code: 0 }); // Success }); +const mockLogShutdown = vi.fn(() => Promise.resolve()); +const mockSpanShutdown = vi.fn(() => Promise.resolve()); + vi.mock("@opentelemetry/exporter-logs-otlp-http", () => ({ OTLPLogExporter: class { export = mockLogExport; - shutdown = vi.fn().mockResolvedValue(undefined); + shutdown = mockLogShutdown; }, })); vi.mock("@opentelemetry/exporter-trace-otlp-http", () => ({ OTLPTraceExporter: class { export = mockSpanExport; - shutdown = vi.fn().mockResolvedValue(undefined); + shutdown = mockSpanShutdown; }, })); @@ -88,6 +91,8 @@ describe("OtelRunTelemetry", () => { beforeEach(() => { mockLogExport.mockClear(); mockSpanExport.mockClear(); + mockLogShutdown.mockClear(); + mockSpanShutdown.mockClear(); }); describe("mapNotificationToLogRecord", () => { @@ -164,11 +169,12 @@ describe("OtelRunTelemetry", () => { name: "error", entry: makeEntry("_posthog/error", { source: "agent_server", + stopReason: "error", error: "boom", }), severityText: "ERROR", - body: "error: boom", - attrs: { error_source: "agent_server" }, + body: "run error", + attrs: { error_source: "agent_server", stop_reason: "error" }, }, { name: "progress", @@ -324,11 +330,29 @@ describe("OtelRunTelemetry", () => { expect(mapNotificationToLogRecord(entry)).toBeNull(); }); - it("caps body length", () => { + // Run errors export provenance only: the raw message is free text that + // can embed prompt or repo content (exception paths, provider errors). + it("never exports the raw error message", () => { const mapped = mapNotificationToLogRecord( makeEntry("_posthog/error", { - source: "agent_server", - error: "x".repeat(5000), + source: "agent_server_crash", + error: "Agent server crashed: ENOENT open '/repos/acme/SECRET/.env'", + }), + ); + + expect(mapped).not.toBeNull(); + expect(JSON.stringify([mapped?.body, mapped?.attributes])).not.toContain( + "SECRET", + ); + }); + + it("caps body length", () => { + const mapped = mapNotificationToLogRecord( + makeEntry("_posthog/progress", { + group: "setup:run-1", + step: "agent", + status: "completed", + label: "x".repeat(5000), }), ); @@ -574,7 +598,7 @@ describe("OtelRunTelemetry", () => { } }); - it("marks failed tools, errored turns, and the errored run", async () => { + it("marks failed tools, interrupted tools, errored turns, and the errored run", async () => { telemetry.append(RUN_ID, makeEntry("session/prompt", {})); telemetry.append( RUN_ID, @@ -592,11 +616,21 @@ describe("OtelRunTelemetry", () => { status: "failed", }), ); + // Still open when the run error below lands. + telemetry.append( + RUN_ID, + sessionUpdate({ + sessionUpdate: "tool_call", + toolCallId: "t2", + kind: "fetch", + }), + ); telemetry.append( RUN_ID, makeEntry("_posthog/error", { source: "agent_server", - error: "gateway exploded", + stopReason: "error", + error: "gateway exploded reading SECRET", }), ); // A turn completion arriving after the error must not flip the run @@ -611,10 +645,37 @@ describe("OtelRunTelemetry", () => { expect(spanByName("tool_call:execute").status.code).toBe( SpanStatusCode.ERROR, ); - expect(spanByName("turn").status.code).toBe(SpanStatusCode.ERROR); + const interrupted = spanByName("tool_call:fetch"); + expect(interrupted.status.code).toBe(SpanStatusCode.ERROR); + expect(interrupted.attributes).toMatchObject({ + tool_status: "interrupted", + }); + const turn = spanByName("turn"); + expect(turn.status.code).toBe(SpanStatusCode.ERROR); + expect(turn.attributes).toMatchObject({ stop_reason: "error" }); const root = spanByName("task_run"); expect(root.status.code).toBe(SpanStatusCode.ERROR); - expect(root.status.message).toBe("gateway exploded"); + expect(root.attributes).toMatchObject({ error_source: "agent_server" }); + // The raw error message is free text; it must not reach span status + // messages or attributes. + const surface = JSON.stringify( + exportedSpans().map((span) => [ + span.name, + span.attributes, + span.status, + ]), + ); + expect(surface).not.toContain("SECRET"); + }); + + it("shuts down logs even when the traces endpoint fails", async () => { + mockSpanShutdown.mockRejectedValueOnce(new Error("traces endpoint down")); + telemetry.append(RUN_ID, makeEntry("_posthog/run_started", {})); + + await expect(telemetry.shutdown()).resolves.toBeUndefined(); + + expect(mockLogShutdown).toHaveBeenCalled(); + expect(exportedLogs().map((log) => log.body)).toContain("run started"); }); it("exports open spans on shutdown even without terminal events", async () => { diff --git a/packages/agent/src/otel-telemetry.ts b/packages/agent/src/otel-telemetry.ts index c0ce887cb3..900c8667c5 100644 --- a/packages/agent/src/otel-telemetry.ts +++ b/packages/agent/src/otel-telemetry.ts @@ -13,6 +13,7 @@ import { POSTHOG_NOTIFICATIONS } from "./acp-extensions"; import { type Attributes, asRecord, + EXPORT_TIMEOUT_MS, entryTime, MAX_BODY_CHARS, strAttr, @@ -192,12 +193,14 @@ export function mapNotificationToLogRecord( return record(INFO, "task complete", method, attrs); } case POSTHOG_NOTIFICATIONS.ERROR: { + // params.error is free text that can embed prompt or repo content + // (exception messages, provider errors), so only its provenance is + // exported. The raw message stays in the session log and on the task + // run's error_message. const attrs: Attributes = {}; strAttr(attrs, "error_source", params.source); strAttr(attrs, "stop_reason", params.stopReason); - const message = - typeof params.error === "string" ? params.error : "unknown error"; - return record(ERROR, `error: ${message}`, method, attrs); + return record(ERROR, "run error", method, attrs); } // POSTHOG_NOTIFICATIONS.CONSOLE is deliberately NOT exported: those are // free-text agent-server diagnostics that interpolate arbitrary data @@ -280,6 +283,7 @@ export class OtelRunTelemetry implements SessionLogSink { const processor = new BatchLogRecordProcessor(exporter, { scheduledDelayMillis: config.flushIntervalMs ?? DEFAULT_FLUSH_INTERVAL_MS, + exportTimeoutMillis: EXPORT_TIMEOUT_MS, }); const resourceAttributes: Attributes = { @@ -337,18 +341,29 @@ export class OtelRunTelemetry implements SessionLogSink { } } + /** + * Best-effort: logs and traces flush independently, so a rejecting or + * hanging traces endpoint can never block or fail the log flush (and vice + * versa). Never rejects. + */ async flush(): Promise { - await Promise.all([ + await Promise.allSettled([ this.loggerProvider.forceFlush(), this.traceBuilder?.flush(), ]); } - /** Ends open spans, flushes batched records, then stops the providers. Idempotent. */ + /** + * Ends open spans, flushes batched records, then stops the providers. + * Idempotent and best-effort: the two providers shut down independently + * and a failure in one never skips the other. Never rejects. + */ async shutdown(): Promise { if (this.shutdownStarted) return; this.shutdownStarted = true; - await this.traceBuilder?.shutdown(); - await this.loggerProvider.shutdown(); + await Promise.allSettled([ + this.traceBuilder?.shutdown(), + this.loggerProvider.shutdown(), + ]); } } diff --git a/packages/agent/src/otel-trace-builder.ts b/packages/agent/src/otel-trace-builder.ts index 6f4872b1a5..1397c49770 100644 --- a/packages/agent/src/otel-trace-builder.ts +++ b/packages/agent/src/otel-trace-builder.ts @@ -17,8 +17,9 @@ import { POSTHOG_NOTIFICATIONS } from "./acp-extensions"; import { type Attributes, asRecord, + EXPORT_TIMEOUT_MS, entryTime, - truncate, + strAttr, usageAttributes, } from "./otel-attributes"; import type { StoredNotification } from "./types"; @@ -77,6 +78,7 @@ export class RunTraceBuilder { new BatchSpanProcessor(exporter, { scheduledDelayMillis: config.flushIntervalMs ?? DEFAULT_FLUSH_INTERVAL_MS, + exportTimeoutMillis: EXPORT_TIMEOUT_MS, }), ], }); @@ -280,20 +282,32 @@ export class RunTraceBuilder { private handleError(params: Record, time: Date): Context { this.rootErrored = true; - this.closeOpenTools(time); - this.closeTurn({ errored: true }, time); - this.rootSpan.setStatus({ - code: SpanStatusCode.ERROR, - message: - typeof params.error === "string" - ? truncate(params.error, 200) - : undefined, - }); + this.closeOpenTools(time, { interrupted: true }); + const stopReason = + typeof params.stopReason === "string" ? params.stopReason : undefined; + this.closeTurn({ stopReason, errored: true }, time); + // params.error is free text that can embed prompt or repo content, so + // only the error's provenance is exported; the raw message stays in the + // session log and on the task run's error_message. + const attrs: Attributes = {}; + strAttr(attrs, "error_source", params.source); + this.rootSpan.setAttributes(attrs); + this.rootSpan.setStatus({ code: SpanStatusCode.ERROR }); return this.rootContext; } - private closeOpenTools(time: Date): void { + /** + * Ends every open tool span. Interrupted (a run error aborted the tool + * mid-flight) marks them errored so APM doesn't show a healthy-looking + * active tool under a failed run; otherwise the outcome is unknown and the + * status stays unset. + */ + private closeOpenTools(time: Date, opts?: { interrupted?: boolean }): void { for (const { span } of this.toolSpans.values()) { + if (opts?.interrupted) { + span.setAttribute("tool_status", "interrupted"); + span.setStatus({ code: SpanStatusCode.ERROR }); + } span.end(time); } this.toolSpans.clear(); diff --git a/packages/agent/src/types.ts b/packages/agent/src/types.ts index 6e8e5a6cc8..8bbffe5e70 100644 --- a/packages/agent/src/types.ts +++ b/packages/agent/src/types.ts @@ -77,8 +77,25 @@ export type OnLogCallback = ( data?: unknown, ) => void; +/** + * @deprecated Ignored. The in-process OTel log transport was removed; run + * telemetry is exported by the agent server, configured via the + * POSTHOG_AGENT_OTEL_LOGS_URL/_TOKEN environment variables. Kept only so + * existing consumers keep compiling; will be removed in a future major. + */ +export interface OtelTransportConfig { + /** PostHog ingest host, e.g., "https://us.i.posthog.com" */ + host: string; + /** Project API key */ + apiKey: string; + /** Override the logs endpoint path (default: /i/v1/logs) */ + logsPath?: string; +} + export interface AgentConfig { posthog?: PostHogAPIConfig; + /** @deprecated Ignored — see {@link OtelTransportConfig}. */ + otelTransport?: OtelTransportConfig; /** Skip session log persistence (e.g. for preview sessions with no real task) */ skipLogPersistence?: boolean; /** Local cache path for instant log loading (e.g., ~/.posthog-code) */ From f3f1bdc1e7b187777db7868528eb96419fe6dbe0 Mon Sep 17 00:00:00 2001 From: Alessandro Pogliaghi Date: Tue, 7 Jul 2026 15:34:54 +0100 Subject: [PATCH 09/11] fix(aio): honor task detail 404 only on cold cache misses The task detail routes redirected away on any 404 from the detail query, even when a cached or list copy of the task exists. Optimistic and cloud-pending tasks are not returnable by the API yet (see the route loader comments), so the unconditional redirect kicked users off tasks they had just created. The detail query still always runs (so a stale cached copy converges on the server's latest run state), but a 404/missing result only triggers the redirect when there is no usable cached task to render. Generated-By: PostHog Code Task-Id: 0c511836-2180-455a-9b58-45df2a0661ec --- .../src/router/routes/code/tasks/$taskId.tsx | 22 +++++++++---------- .../website/$channelId/tasks/$taskId.tsx | 5 ++++- 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/packages/ui/src/router/routes/code/tasks/$taskId.tsx b/packages/ui/src/router/routes/code/tasks/$taskId.tsx index 898483217e..ffef3b0592 100644 --- a/packages/ui/src/router/routes/code/tasks/$taskId.tsx +++ b/packages/ui/src/router/routes/code/tasks/$taskId.tsx @@ -3,7 +3,6 @@ import { TaskDetail } from "@posthog/ui/features/task-detail/components/TaskDeta import { getCachedTask, getCachedTaskDetail, - isTaskDetailNotFoundError, taskDetailQuery, } from "@posthog/ui/features/tasks/queries"; import { pickFreshestTask } from "@posthog/ui/features/tasks/taskFreshness"; @@ -41,25 +40,24 @@ function TaskDetailRoute() { const fromList = tasks?.find((t) => t.id === taskId); const initialTask = pickFreshestTask(fromList, loaderTask); - // Cold deep-link / URL restore: nothing cached. Fetch the single task here so - // a hang or 404 only affects this view's spinner, never the router. - const needsFetch = !initialTask; + // Always fetch so a stale cached copy converges on the server's latest run + // state; render whichever copy is freshest. const { data: fetched, - error, isError, isSuccess, } = useQuery(taskDetailQuery(taskId)); const task = pickFreshestTask(fetched, initialTask); - // Task doesn't exist (deleted, 404, or stale deep link): the cold fetch - // settled with an error or empty result. Redirect to the new-task screen - // rather than spin forever — matches the old navigationStore.hydrateTask. - if ( - isTaskDetailNotFoundError(error) || - (needsFetch && (isError || (isSuccess && !fetched))) - ) { + // Cold deep-link / URL restore with nothing cached: if the fetch settled + // with an error or empty result, redirect to the new-task screen rather + // than spin forever — matches the old navigationStore.hydrateTask. While a + // cached/list copy exists, a 404 is NOT authoritative (optimistic and + // cloud-pending tasks aren't returnable by the API yet — see the loader + // comment), so never redirect away from a usable task. + const needsFetch = !initialTask; + if (needsFetch && (isError || (isSuccess && !fetched))) { return ; } diff --git a/packages/ui/src/router/routes/website/$channelId/tasks/$taskId.tsx b/packages/ui/src/router/routes/website/$channelId/tasks/$taskId.tsx index 08a1281509..133e79e5c0 100644 --- a/packages/ui/src/router/routes/website/$channelId/tasks/$taskId.tsx +++ b/packages/ui/src/router/routes/website/$channelId/tasks/$taskId.tsx @@ -73,7 +73,10 @@ function ChannelTaskDetailRoute() { const task = pickFreshestTask(fetched, initialTask); - if (isTaskDetailNotFoundError(error)) { + // While a cached/list copy exists, a 404 is NOT authoritative (optimistic + // and cloud-pending tasks aren't returnable by the API yet — see the loader + // comment), so only treat the task as gone when nothing cached is usable. + if (!initialTask && isTaskDetailNotFoundError(error)) { return ; } From 0bf0babe46d31862f277101e3ffef9d3e7f5da8c Mon Sep 17 00:00:00 2001 From: Alessandro Pogliaghi Date: Wed, 8 Jul 2026 15:00:14 +0100 Subject: [PATCH 10/11] fix(agent): end run telemetry at the in-process terminal point Local verification of the telemetry branch showed logs and the turn span landing in ClickHouse but never the task_run root span, even after the run reached completed and extra delay. Root cause: the root span only ended in cleanupSession, which is only reachable via SIGTERM or the close command - and sandbox teardown never delivers SIGTERM to agent-server, because it is an exec'd process (docker stop signals only the container's PID 1; Modal terminate is immediate). The root span was still open when the process was killed, so it never exported. The turn span survived only because it ended mid-run and the 2s batch exporter got it out. Fix: end telemetry eagerly at the run's in-process terminal points instead of relying on teardown: - finalizeRunTelemetry: full telemetry shutdown (ends the root span with the resolved status and drains both exporters) when a background run's initial or resume prompt settles - the run is over in-sandbox at that point; the workflow marks the terminal status and destroys the sandbox right after. - signalTaskComplete: upgrade the terminal-failure flush to a full shutdown, after the error mirror is appended, so failed runs export a root span with ERROR status. - cleanupSession remains the path for interactive close. Known limitation, now documented: an interactive session ended by hard teardown (e.g. inactivity timeout) loses the root span; its turn/tool spans and logs still assemble under the same trace id. Tests: order assertion that the error mirror lands before the terminal shutdown, and a parameterized check that finalizeRunTelemetry fires for background runs only. The full agent-server + telemetry suites pass (161). Stacked on posthog-code/agent-run-otel-telemetry (pure fast-forward). Generated-By: PostHog Code Task-Id: 0c511836-2180-455a-9b58-45df2a0661ec --- REPORT.md | 10 +- packages/agent/README.md | 2 + .../agent/src/server/agent-server.test.ts | 118 ++++++++++++++++++ packages/agent/src/server/agent-server.ts | 31 ++++- 4 files changed, 153 insertions(+), 8 deletions(-) diff --git a/REPORT.md b/REPORT.md index 08c0cc32bc..8fc22a57e6 100644 --- a/REPORT.md +++ b/REPORT.md @@ -67,7 +67,7 @@ Bodies are capped at 2000 chars; free-text attribute values at 200 chars (the `l ### APM trace (one per run) -- `task_run` root span (kind SERVER): opened at session init, closed at session cleanup. Status is resolved at shutdown from the latest turn outcome (the sandbox never emits `task_complete` for successful runs — the terminal "completed" status is decided by the workflow outside — so the last turn is the in-sandbox success signal): OK when the last turn ended with `end_turn`, ERROR on `_posthog/error` (an error always wins, later turn completions cannot flip it back; `error_source` lands as a root-span attribute while the raw message is withheld — see the error row above) or when the last turn stopped with `error`, unset otherwise (cancelled / refused / timed out / no completed turns). Resolving at shutdown rather than per-turn means an early clean turn cannot leave a stale OK on a run whose final turn was cancelled. +- `task_run` root span (kind SERVER): opened at session init, closed at the run's in-process terminal point — a background run's prompt settling (`finalizeRunTelemetry`), a terminal failure (`signalTaskComplete`), or session cleanup (interactive `close`). Status is resolved at shutdown from the latest turn outcome (the sandbox never emits `task_complete` for successful runs — the terminal "completed" status is decided by the workflow outside — so the last turn is the in-sandbox success signal): OK when the last turn ended with `end_turn`, ERROR on `_posthog/error` (an error always wins, later turn completions cannot flip it back; `error_source` lands as a root-span attribute while the raw message is withheld — see the error row above) or when the last turn stopped with `error`, unset otherwise (cancelled / refused / timed out / no completed turns). Resolving at shutdown rather than per-turn means an early clean turn cannot leave a stale OK on a run whose final turn was cancelled. - `turn` child spans: opened on each ACP `session/prompt` (used purely as a boundary marker; its content is never read), closed on `_posthog/turn_complete`; attributes `turn_index`, `stop_reason`, plus the turn's token counts and `cost_usd` lifted from usage updates, so APM can rank slow or expensive turns directly. - `tool_call:` grandchild spans (`execute`, `read`, `edit`, ...): opened on `tool_call`, closed on the terminal `tool_call_update`; status ERROR on `failed`; attributes `tool_call_id`, `tool_kind`, `tool_status`. Per-kind span names stay low-cardinality and make APM latency breakdowns by tool kind useful. - Robustness: orphaned spans are closed (status unset) and exported at shutdown; a new prompt while a turn is open closes the stale turn; duplicate `tool_call` events are idempotent; a run error cascades ERROR status to the open turn and the root, and closes still-open tool spans as ERROR with `tool_status=interrupted` so APM never shows a healthy-looking active tool under a failed run. @@ -76,8 +76,8 @@ Bodies are capped at 2000 chars; free-text attribute values at 200 chars (the `l ### Delivery timing Telemetry is near-realtime, not end-of-turn: records are created the moment each notification flows through the writer and batched for at most 2 s (`BatchLogRecordProcessor` / `BatchSpanProcessor`, `scheduledDelayMillis` 2000). -Spans export when they end (tools mid-turn, turns at `turn_complete`, root at cleanup). -Flush safety nets: an explicit flush after a terminal error in `signalTaskComplete`, a full shutdown-flush in `cleanupSession` (which SIGTERM reaches via `stop()`), so sandbox teardown cannot eat the tail of a run's telemetry. +Spans export when they end (tools mid-turn, turns at `turn_complete`, root at the run's terminal point). +Sandbox teardown can NOT be a flush point: agent-server is an exec'd process inside the sandbox, so `docker stop` signals only the container's PID 1 and Modal terminate is immediate — the process's SIGTERM handler never runs and anything still queued (or a still-open root span) is lost. Telemetry is therefore ended eagerly at the in-process terminal points: `finalizeRunTelemetry` when a background run's prompt settles, a full shutdown after a terminal failure in `signalTaskComplete`, and `cleanupSession` for interactive `close`. Known limitation: an interactive session ended by hard teardown (e.g. inactivity timeout) loses the root span; its turn/tool spans and logs still assemble under the same trace id. Flush and shutdown are best-effort and per-signal independent (`Promise.allSettled`), and every export is capped at 5 s (`exportTimeoutMillis`, down from the SDK's 30 s default), so a rejecting or hanging traces endpoint can neither starve log delivery nor hold up session cleanup. ## Changes in PostHog/code (this branch) @@ -86,11 +86,11 @@ Flush and shutdown are best-effort and per-signal independent (`Promise.allSettl - `packages/agent/src/otel-trace-builder.ts` (new): `RunTraceBuilder`, the span state machine described above; `handle(entry)` returns the context each log record should attach to. - `packages/agent/src/otel-attributes.ts` (new): shared pure helpers (`strAttr`, `numAttr`, `usageAttributes`, truncation, caps). - `packages/agent/src/session-log-writer.ts`: optional `sinks: SessionLogSink[]`, teed in `appendRawLine` after the entry is built; a throwing sink warns once and can never break product log persistence; message chunks never reach sinks. `SessionContext` moved here from the otel module. -- `packages/agent/src/server/agent-server.ts`: builds the telemetry per session from config (`createRunTelemetry`), passes it as the writer sink, stores it on the session, shuts it down in `cleanupSession`, flushes after terminal errors, and mirrors `enqueueTaskTerminalEvent` payloads into it directly (terminal `_posthog/error` events bypass `SessionLogWriter`, and a failed run is exactly what telemetry must record). Fatal crashes (`reportFatalError`, the uncaught-exception/unhandled-rejection path) also mirror an error record (`error_source=agent_server_crash`) and shut telemetry down, so hard process deaths reach the telemetry project instead of vanishing. +- `packages/agent/src/server/agent-server.ts`: builds the telemetry per session from config (`createRunTelemetry`), passes it as the writer sink, stores it on the session, and ends it at the run's in-process terminal points: `finalizeRunTelemetry` (full shutdown when a background run's initial/resume prompt settles — verified necessary because sandbox teardown never delivers SIGTERM to the exec'd process, so waiting for `cleanupSession` left the root span unexported), a shutdown after terminal failures in `signalTaskComplete`, and `cleanupSession` for interactive `close`. `enqueueTaskTerminalEvent` payloads are mirrored into it directly (terminal `_posthog/error` events bypass `SessionLogWriter`, and a failed run is exactly what telemetry must record). Fatal crashes (`reportFatalError`, the uncaught-exception/unhandled-rejection path) also mirror an error record (`error_source=agent_server_crash`) and shut telemetry down, so hard process deaths reach the telemetry project instead of vanishing. - `packages/agent/src/server/bin.ts` + `server/types.ts`: zod-validated env `POSTHOG_AGENT_OTEL_LOGS_URL`, `POSTHOG_AGENT_OTEL_LOGS_TOKEN`, `POSTHOG_AGENT_OTEL_TRACES_URL` → `AgentServerConfig.otelLogsUrl/otelLogsToken/otelTracesUrl`. Telemetry is off unless the logs pair is set; spans additionally require the traces URL (per-signal kill switch). - `packages/agent/src/types.ts`: the February `OtelTransportConfig`/`AgentConfig.otelTransport` remain as `@deprecated`, ignored stubs — `@posthog/agent` is a published package, so removing exported types is an API break reserved for a major. - Dependencies: `@opentelemetry/api`, `@opentelemetry/sdk-trace-base`, `@opentelemetry/exporter-trace-otlp-http`, version-aligned with the existing logs SDK (0.208.x experimental / 2.x stable line). -- Tests (74 passing): parameterized log-mapping matrix, hard privacy tests asserting exported payloads never contain tool args/titles/output or raw error messages, per-user resource attributes, session-mismatch guard, never-throws guard, sink isolation in `SessionLogWriter`, and five trace tests (span tree + statuses + attributes, log⇄span id correlation, error cascade incl. interrupted tools, orphan export on shutdown, log shutdown isolated from a failing traces endpoint). +- Tests (74 passing in the telemetry suites, plus agent-server terminal-point tests): parameterized log-mapping matrix, hard privacy tests asserting exported payloads never contain tool args/titles/output or raw error messages, per-user resource attributes, session-mismatch guard, never-throws guard, sink isolation in `SessionLogWriter`, five trace tests (span tree + statuses + attributes, log⇄span id correlation, error cascade incl. interrupted tools, orphan export on shutdown, log shutdown isolated from a failing traces endpoint), and agent-server tests asserting the error mirror lands before the terminal shutdown and that `finalizeRunTelemetry` fires for background runs only. - `packages/agent/README.md`: documents the env vars and behavior. ## Changes in PostHog/posthog (companion branch) diff --git a/packages/agent/README.md b/packages/agent/README.md index 985946a92e..77775935f7 100644 --- a/packages/agent/README.md +++ b/packages/agent/README.md @@ -174,6 +174,8 @@ Optional run telemetry (the logs pair must both be set, otherwise telemetry stay When set, `AgentServer` ships an allowlisted metadata subset of the session log (run/turn/tool lifecycle, usage, error provenance — never message content, tool arguments, or raw error text; see `src/otel-telemetry.ts`) to PostHog Logs, tagged with `service.name=posthog-code-agent` and `run_id`/`task_id`/`team_id`/`user_id`/`distinct_id` resource attributes so cloud runs are filterable per user. With the traces URL set, each run also produces an APM trace (`task_run` root span, a `turn` span per prompt, a `tool_call:` span per tool call; see `src/otel-trace-builder.ts`), and log records carry the matching trace/span ids so Logs and APM cross-link. +The `task_run` root span is ended and exported at the run's in-process terminal point — a background run's prompt settling, a terminal failure, or session cleanup (`close`). It cannot wait for teardown: agent-server is an exec'd process inside the sandbox, so `docker stop` / Modal terminate kill it without SIGTERM ever arriving, and a span still open at that moment is lost. Interactive sessions that end via hard teardown (e.g. inactivity timeout) therefore lose the root span; turn/tool spans and logs still assemble under the same trace id. + ## Agent SDK The `Agent` class (`src/agent.ts`) is the entrypoint for local/programmatic usage. It handles LLM gateway configuration, log writer setup, and model filtering — then delegates to `createAcpConnection()`. diff --git a/packages/agent/src/server/agent-server.test.ts b/packages/agent/src/server/agent-server.test.ts index bf5aab9734..1741b595ee 100644 --- a/packages/agent/src/server/agent-server.test.ts +++ b/packages/agent/src/server/agent-server.test.ts @@ -908,6 +908,124 @@ describe("AgentServer HTTP Mode", () => { ); }); + // Sandbox teardown kills the exec'd agent-server without SIGTERM, so the + // trace's root span only exports if telemetry is shut down at the run's + // in-process terminal points. + it("shuts down telemetry after mirroring the terminal failure record", async () => { + const order: string[] = []; + const testServer = new AgentServer({ + port, + jwtPublicKey: TEST_PUBLIC_KEY, + repositoryPath: repo.path, + apiUrl: "http://localhost:8000", + apiKey: "test-api-key", + projectId: 1, + mode: "interactive", + taskId: "test-task-id", + runId: "test-run-id", + }) as unknown as { + session: { + payload: { run_id: string }; + logWriter: { flush: ReturnType }; + telemetry: { + append: ReturnType; + shutdown: ReturnType; + }; + }; + eventStreamSender: { + enqueue: (event: Record) => void; + stop: () => Promise; + }; + posthogAPI: { + updateTaskRun: ( + taskId: string, + runId: string, + payload: Record, + ) => Promise; + }; + signalTaskComplete( + payload: JwtPayload, + stopReason: string, + errorMessage?: string, + ): Promise; + }; + testServer.eventStreamSender = { + enqueue: vi.fn(), + stop: vi.fn(async () => {}), + }; + testServer.posthogAPI = { + updateTaskRun: vi.fn(async () => ({})), + }; + testServer.session = { + payload: { run_id: "run-1" }, + logWriter: { flush: vi.fn(async () => {}) }, + telemetry: { + append: vi.fn(() => { + order.push("append"); + }), + shutdown: vi.fn(async () => { + order.push("shutdown"); + }), + }, + }; + + await testServer.signalTaskComplete( + { + run_id: "run-1", + task_id: "task-1", + team_id: 1, + user_id: 1, + distinct_id: "distinct-id", + mode: "background", + }, + "error", + "boom", + ); + + // The error mirror must land before shutdown so the root span exports + // with ERROR status. + expect(order).toEqual(["append", "shutdown"]); + }); + + it.each([ + { mode: "background" as const, shutdownCalls: 1 }, + { mode: "interactive" as const, shutdownCalls: 0 }, + ])( + "finalizeRunTelemetry shuts down telemetry only for $mode runs", + async ({ mode, shutdownCalls }) => { + const testServer = new AgentServer({ + port, + jwtPublicKey: TEST_PUBLIC_KEY, + repositoryPath: repo.path, + apiUrl: "http://localhost:8000", + apiKey: "test-api-key", + projectId: 1, + mode: "interactive", + taskId: "test-task-id", + runId: "test-run-id", + }) as unknown as { + session: { telemetry: { shutdown: ReturnType } }; + finalizeRunTelemetry(payload: JwtPayload): Promise; + }; + testServer.session = { + telemetry: { shutdown: vi.fn(async () => {}) }, + }; + + await testServer.finalizeRunTelemetry({ + run_id: "run-1", + task_id: "task-1", + team_id: 1, + user_id: 1, + distinct_id: "distinct-id", + mode, + }); + + expect(testServer.session.telemetry.shutdown).toHaveBeenCalledTimes( + shutdownCalls, + ); + }, + ); + function createFailureTestServer() { const appendRawLine = vi.fn(); const testServer = new AgentServer({ diff --git a/packages/agent/src/server/agent-server.ts b/packages/agent/src/server/agent-server.ts index ff0bf79592..89170bd176 100644 --- a/packages/agent/src/server/agent-server.ts +++ b/packages/agent/src/server/agent-server.ts @@ -1844,6 +1844,8 @@ export class AgentServer { if (result.stopReason === "end_turn") { await this.relayAgentResponse(payload); } + + await this.finalizeRunTelemetry(payload); } catch (error) { this.logger.error("Failed to send initial task message", error); if (this.session) { @@ -2060,6 +2062,8 @@ export class AgentServer { if (result.stopReason === "end_turn") { await this.relayAgentResponse(payload); } + + await this.finalizeRunTelemetry(payload); } catch (error) { this.logger.error(`Failed to send ${logLabel.toLowerCase()}`, error); if (this.session) { @@ -3303,6 +3307,25 @@ ${signedCommitInstructions}${prLinkInstructions}${shellEfficiencyInstructions} } } + /** + * Ends the run's telemetry (root span + final flush) at the in-sandbox + * terminal point of a background run. Sandbox teardown cannot be relied on + * for this: agent-server is an exec'd process inside the sandbox, so + * `docker stop` signals only the container's PID 1 and Modal terminate is + * immediate — the SIGTERM handler (and thus cleanupSession) never runs, and + * an unended root span would never export. Once the background prompt + * settles the run is over in-sandbox; the workflow marks the terminal + * status and destroys the sandbox right after. + */ + private async finalizeRunTelemetry(payload: JwtPayload): Promise { + if (this.getEffectiveMode(payload) !== "background") return; + try { + await this.session?.telemetry?.shutdown(); + } catch (error) { + this.logger.debug("Failed to finalize run telemetry", error); + } + } + private async signalTaskComplete( payload: JwtPayload, stopReason: string, @@ -3348,9 +3371,11 @@ ${signedCommitInstructions}${prLinkInstructions}${shellEfficiencyInstructions} } finally { await this.emitRtkSavings(); await this.eventStreamSender?.stop(); - // The sandbox can be torn down right after the failed status lands; - // don't leave the terminal record sitting in the OTel batch queue. - await this.session?.telemetry?.flush().catch(() => {}); + // The run is terminal and the sandbox is torn down right after — and + // teardown kills this exec'd process without SIGTERM, so this is the + // last chance to end the root span and drain the OTel queues. The + // error mirror was appended above, so the root span exports as ERROR. + await this.session?.telemetry?.shutdown().catch(() => {}); } } From b4d42d402103e9e9003d471d481c74464d259983 Mon Sep 17 00:00:00 2001 From: Alessandro Pogliaghi Date: Tue, 14 Jul 2026 10:30:24 +0100 Subject: [PATCH 11/11] fix(aio): restore terminal permission hydration --- packages/core/src/sessions/sessionService.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/core/src/sessions/sessionService.ts b/packages/core/src/sessions/sessionService.ts index d0c72e327a..43e868a88c 100644 --- a/packages/core/src/sessions/sessionService.ts +++ b/packages/core/src/sessions/sessionService.ts @@ -4502,6 +4502,8 @@ export class SessionService { const effectiveLineCount = Math.max(totalLineCount, rawEntries.length); if (isTerminalRun) { if ((session.processedLineCount ?? 0) >= effectiveLineCount) { + this.surfacePersistedPendingPermissions(taskRunId, rawEntries); + this.pendingPermissionHydratedRuns.add(taskRunId); return; } } else if (