chore: sync implementation report with review fixes

Generated-By: PostHog Code
Task-Id: 0c511836-2180-455a-9b58-45df2a0661ec

Author: tatoalo

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:<kind>` 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.