feat(telemetry): one session id and one trace per turn (RPT-057)

[Nivesh353]

What — Every model request now carries X-Session-Id, and every request within a turn shares a traceparent.

Why — RPT-057: one gitagent run produced several unrelated traces in Studio with no way to correlate them or total consumption.

How — loadAgent() takes an optional session id (CLI --session-id, SDK sessionId) and puts it on the model's headers, cloned so the shared pi-ai registry object isn't mutated. startTurnTrace() sets a fresh traceparent before each turn.

Verified — Against dev: two traces sharing one session id, then a single trace of 12 spans once the server-side change landed

[shreyas-lyzr]

Five focused items across correctness, logic, and security — two worth addressing before ship.

1. Guard in startTurnTrace is inverted (logic bug)

The guard is if (_initialized || !model) return; — this makes the function a no-op when OTel is active (_initialized === true) and writes a manual traceparent when telemetry is disabled. That is the opposite of the documented intent. The docstring says: when OTel is initialised, UndiciInstrumentation already injects traceparent from the active span automatically and a manually-written header would fight it — so skip when enabled, act when disabled. The correct guard is if (!_initialized || !model) return;.

2. startTurnTrace mutates the model object in place across turns

m.headers = { ...(m.headers ?? {}), traceparent } writes directly to loaded.model. loader.ts explicitly clones the registry object to prevent exactly this kind of cross-run contamination. In the REPL each turn overwrites the previous traceparent so it works serially, but in a concurrent SDK scenario (two query() calls in the same process) both writers race on the same object. Consider returning a headers delta and letting the caller compose it, or documenting the single-threaded assumption clearly.

3. _replActive declaration sits below the call that references it

let _replActive = false is declared after main() is called at module level. With let there is no hoisting, so the declaration is reached only because main() is async and .finally() runs after the event loop tick. It works in practice but is fragile to read. Moving the declaration above main() costs one line and removes the confusion.

4. Telemetry flush on single-shot error path — appears safe, but non-obvious

When agent.prompt() throws in single-shot mode, the error propagates through main().catch() which calls process.exit(1) — but the .finally(() => { if (!_replActive) shutdownTelemetry() }) on the chain fires first, so telemetry is flushed. This is correct; a brief comment noting it is intentional would save the next reader from worrying about it.

5. Security pass: clean

No new runtime dependencies. The only new import is randomBytes from Node's built-in crypto. X-Session-Id carries a UUID; traceparent is a W3C correlation header — neither is a credential. No injection, SSRF, IDOR, or authz surface touched. No secrets in the diff.

[shreyas-lyzr]

All three findings from the previous round are addressed.

The logic in startTurnTrace (if (_initialized || !model) return) was not a bug — the new JSDoc makes the intent unambiguous: when OTel is active, undici already injects traceparent from the active span, so this function deliberately yields to it. The guard is correct.

The write-through to model.headers is now clearly documented as safe: loadAgent clones the model off the shared registry, every query() load gets its own copy, concurrent runs never share an object, and turns within a run are sequential. The mutation is intentional.

_replActive is now hoisted above main(), with a comment explaining why the REPL outlives main()'s promise.

Security pass: no changes to deps, no secrets, no injection/authz surface touched. Clean.