From 922f518b48449502c1166247a2d830cba27415b3 Mon Sep 17 00:00:00 2001 From: mintaka Date: Wed, 26 Aug 2026 02:27:25 -0400 Subject: [PATCH 1/2] feat(compass-agent): activate loop OpenTelemetry in cli.ts main() (RIG-2508 T1) Implements task T1 of the frozen record docs/designs/platform/compass-agent-loop-otel/design.md: activate the OMP agent loop's native GenAI-semconv OpenTelemetry (invoke_agent / chat / execute_tool spans) by passing `telemetry: {}` to createAgentSession, gated hard on an OTLP endpoint being configured. - `isTelemetryEndpointConfigured(env)`: the pre-registration gate mirroring `initTelemetryExport` (endpoint present via OTEL_EXPORTER_OTLP_TRACES_ENDPOINT or the base OTEL_EXPORTER_OTLP_ENDPOINT, kill-switches clear). - Enabled path only: default OTEL_SERVICE_NAME (never clobber a deployer value), append `compass.session.id=` to OTEL_RESOURCE_ATTRIBUTES as the cross-signal join key (Decision 3a), then `initTelemetryExport()`. Order is load-bearing: env before registration. - `telemetry: {}` on the session options keyed off the authoritative post-registration `isTelemetryExportEnabled()` check; the key is omitted entirely when off. - Off by default: with no OTLP endpoint the whole block is skipped, so process.env is unmutated and the session build is bit-identical to a no-telemetry container (Global Constraints; F2). - Telemetry hooks injected at the MainDeps seam so tests assert the gating / env / option logic without running the real global-provider registration (F3 test isolation). Zero transport change (OQ1 ruled (b)); no new deps, no bun.lock change. Co-authored-by: Matt Wilkinson --- packages/compass-agent/src/cli.test.ts | 227 +++++++++++++++++++++++++ packages/compass-agent/src/cli.ts | 96 +++++++++++ 2 files changed, 323 insertions(+) diff --git a/packages/compass-agent/src/cli.test.ts b/packages/compass-agent/src/cli.test.ts index 675dd97e5..52ae92eac 100644 --- a/packages/compass-agent/src/cli.test.ts +++ b/packages/compass-agent/src/cli.test.ts @@ -39,6 +39,7 @@ import { createSeedApiKeyResolver, deriveLitellmMcpUrl, envFilePath, + isTelemetryEndpointConfigured, type MainDeps, main, parseEnvFile, @@ -567,11 +568,25 @@ function fakeSession(opts: { promptError?: Error } = {}): FakeSession { // `createTeeSessionStorage`, writing under the per-test scratch HOME (pinned in // beforeEach) — so `main`'s full composition (build sink → tee storage → // SessionManager.create) is exercised, not stubbed. +// +// The telemetry seam is a RECORDING NO-OP (never the real `initTelemetryExport`, +// which registers a live global TracerProvider + OTLP exporter with no teardown +// and would poison every later test in this shared process — design record F3). +// It reports enabled iff an OTLP endpoint is configured at call time, so a test +// that sources an endpoint still exercises main's gating without real +// registration. The dedicated telemetry tests below use their own recording +// seam to assert the calls. function deps(session: FakeSession, transport: RunnerTransport): MainDeps { return { createSession: () => Promise.resolve({ session: session as unknown as AgentSession }), createTransport: () => transport, + telemetry: { + init: () => Promise.resolve(), + isEnabled: () => + process.env.OTEL_EXPORTER_OTLP_TRACES_ENDPOINT !== undefined || + process.env.OTEL_EXPORTER_OTLP_ENDPOINT !== undefined, + }, }; } @@ -1431,6 +1446,10 @@ describe("main sources $HOME/.compass/env into process.env", () => { "SOME_TEST_KEY", "COMPASS_MODEL", "OTEL_EXPORTER_OTLP_ENDPOINT", + // main's enabled-path telemetry activation writes these when an endpoint is + // sourced (the OTEL-endpoint test below); save+restore so they never leak. + "OTEL_SERVICE_NAME", + "OTEL_RESOURCE_ATTRIBUTES", "COMPASS_FUTURE_VAR", "LITELLM_BASE_URL", "LITELLM_MCP_URL", @@ -1588,6 +1607,214 @@ describe("main sources $HOME/.compass/env into process.env", () => { }); }); +// ── main(): loop OpenTelemetry activation ──────────────────────────────────── +// +// design docs/designs/platform/compass-agent-loop-otel/design.md T1. These run +// over the MainDeps composition seam with a RECORDING telemetry seam — NEVER the +// real `initTelemetryExport`, which registers a live global TracerProvider + a +// real OTLP exporter with no teardown and would poison every later test in this +// shared process (design record F3). They assert exactly what cli.ts owns: the +// endpoint gate, the enabled-path env writes (defaulted service name + appended +// join key), the bit-identical inertness when off, and the gated `telemetry` +// session option. Span correctness is OMP's own suite; the real registration is +// left to a spawned-subprocess smoke (not run in-process, per F3). +describe("main activates loop OpenTelemetry", () => { + // Every OTEL_* key these tests read or main may write, saved+restored so a + // leaked var can never flake a later test — the savedHome/TOUCHED_KEYS pattern. + const OTEL_KEYS = [ + "OTEL_EXPORTER_OTLP_ENDPOINT", + "OTEL_EXPORTER_OTLP_TRACES_ENDPOINT", + "OTEL_SERVICE_NAME", + "OTEL_RESOURCE_ATTRIBUTES", + "OTEL_SDK_DISABLED", + "OTEL_TRACES_EXPORTER", + ] as const; + let savedOtel: Record = {}; + beforeEach(() => { + savedOtel = {}; + for (const key of OTEL_KEYS) { + savedOtel[key] = process.env[key]; + delete process.env[key]; + } + }); + afterEach(() => { + for (const key of OTEL_KEYS) { + const prev = savedOtel[key]; + if (prev === undefined) delete process.env[key]; + else process.env[key] = prev; + } + }); + + // A recording telemetry seam + the captured createSession options. `isEnabled` + // mirrors the real module: it reports true only AFTER init() ran (a real + // provider registered), so the option gate keys off registration, not the + // endpoint. The seam records call order so a test can pin env-before-init. + interface TelemetrySpy { + calls: string[]; + telemetryOption: unknown; + hasTelemetryKey: boolean; + } + function telemetryDeps( + session: FakeSession, + transport: RunnerTransport, + spy: TelemetrySpy, + ): MainDeps { + let registered = false; + return { + createSession: (options) => { + spy.telemetryOption = options.telemetry; + spy.hasTelemetryKey = "telemetry" in options; + return Promise.resolve({ + session: session as unknown as AgentSession, + }); + }, + createTransport: () => transport, + telemetry: { + init: () => { + // Capture the env the real provider would read at registration — + // pinning that the defaults are in place BEFORE init runs. + spy.calls.push( + `init:${process.env.OTEL_SERVICE_NAME}:${process.env.OTEL_RESOURCE_ATTRIBUTES}`, + ); + registered = true; + return Promise.resolve(); + }, + isEnabled: () => registered, + }, + }; + } + + // The predicate main gates on, exercised directly — the endpoint contract in + // isolation from the composition (the enum-list + kill-switch branches are + // awkward to reach through the env file, and this is the authoritative gate). + describe("isTelemetryEndpointConfigured", () => { + test("false when no endpoint is set", () => { + expect(isTelemetryEndpointConfigured({})).toBe(false); + }); + test("true for the base endpoint", () => { + expect( + isTelemetryEndpointConfigured({ + OTEL_EXPORTER_OTLP_ENDPOINT: "http://collector:4318", + }), + ).toBe(true); + }); + test("true for the traces-specific endpoint alone", () => { + expect( + isTelemetryEndpointConfigured({ + OTEL_EXPORTER_OTLP_TRACES_ENDPOINT: "http://collector:4318/v1/traces", + }), + ).toBe(true); + }); + test("false when OTEL_SDK_DISABLED is true, even with an endpoint", () => { + expect( + isTelemetryEndpointConfigured({ + OTEL_EXPORTER_OTLP_ENDPOINT: "http://collector:4318", + OTEL_SDK_DISABLED: "TRUE", + }), + ).toBe(false); + }); + test("false when OTEL_TRACES_EXPORTER names none, even with an endpoint", () => { + expect( + isTelemetryEndpointConfigured({ + OTEL_EXPORTER_OTLP_ENDPOINT: "http://collector:4318", + OTEL_TRACES_EXPORTER: "otlp,none", + }), + ).toBe(false); + }); + }); + + // Endpoint UNSET ⇒ true bit-identical inertness (F2): NO `telemetry` key on + // the options AND process.env is unmutated. Snapshot the two enabled-path keys + // before and after so a stray write (an ungated mutation) reddens. Non-vacuity: + // dropping the `if (isTelemetryEndpointConfigured(...))` gate writes + // OTEL_SERVICE_NAME here → red. + test("endpoint unset ⇒ no telemetry key and process.env is unmutated", async () => { + const beforeName = process.env.OTEL_SERVICE_NAME; + const beforeAttrs = process.env.OTEL_RESOURCE_ATTRIBUTES; + const spy: TelemetrySpy = { + calls: [], + telemetryOption: "SENTINEL", + hasTelemetryKey: true, + }; + await main( + { HOME: scratch() }, + telemetryDeps( + fakeSession(), + fakeCarrier(emptyLog(), { control: emptyControlStream }), + spy, + ), + ); + expect(spy.calls).toEqual([]); + expect(spy.hasTelemetryKey).toBe(false); + expect(spy.telemetryOption).toBeUndefined(); + expect(process.env.OTEL_SERVICE_NAME).toBe(beforeName); + expect(process.env.OTEL_RESOURCE_ATTRIBUTES).toBe(beforeAttrs); + }); + + // Endpoint SET ⇒ init() ran with the env defaults in place, and the options + // carry `telemetry: {}`. Pins: OTEL_SERVICE_NAME defaulted, compass.session.id + // appended to OTEL_RESOURCE_ATTRIBUTES, and — via the recorded init call — that + // both were set BEFORE init ran (the load-bearing order). + test("endpoint set ⇒ env defaults set before init, and telemetry:{} on the options", async () => { + process.env.OTEL_EXPORTER_OTLP_ENDPOINT = "http://collector:4318"; + const spy: TelemetrySpy = { + calls: [], + telemetryOption: "SENTINEL", + hasTelemetryKey: false, + }; + await main( + { HOME: scratch() }, + telemetryDeps( + fakeSession(), + fakeCarrier(emptyLog(), { control: emptyControlStream }), + spy, + ), + ); + expect(spy.hasTelemetryKey).toBe(true); + expect(spy.telemetryOption).toEqual({}); + expect(process.env.OTEL_SERVICE_NAME).toBe("compass-agent"); + // The join key was appended; the session id is minted, so assert the shape, + // not a fixed id. + expect(process.env.OTEL_RESOURCE_ATTRIBUTES).toMatch( + /^compass\.session\.id=.+/, + ); + // init ran exactly once, and it observed the defaults already in place — + // the load-bearing env-before-registration order. + expect(spy.calls).toHaveLength(1); + expect(spy.calls[0]).toMatch(/^init:compass-agent:compass\.session\.id=.+/); + }); + + // Deployer-set values are PRESERVED: ??= no-ops the service name, and the join + // key APPENDS after the deployer's existing OTEL_RESOURCE_ATTRIBUTES (never + // clobbers it — Decision 3a). Non-vacuity: a `=` instead of `??=`, or an + // assignment instead of an append, reds one of these. + test("deployer-set OTEL_SERVICE_NAME / OTEL_RESOURCE_ATTRIBUTES are preserved", async () => { + process.env.OTEL_EXPORTER_OTLP_ENDPOINT = "http://collector:4318"; + process.env.OTEL_SERVICE_NAME = "deployer-name"; + process.env.OTEL_RESOURCE_ATTRIBUTES = "deployment.environment=prod"; + const spy: TelemetrySpy = { + calls: [], + telemetryOption: undefined, + hasTelemetryKey: false, + }; + await main( + { HOME: scratch() }, + telemetryDeps( + fakeSession(), + fakeCarrier(emptyLog(), { control: emptyControlStream }), + spy, + ), + ); + // ??= left the deployer's name untouched. + expect(process.env.OTEL_SERVICE_NAME).toBe("deployer-name"); + // The join key was appended AFTER the deployer's value, comma-joined. + expect(process.env.OTEL_RESOURCE_ATTRIBUTES).toMatch( + /^deployment\.environment=prod,compass\.session\.id=.+/, + ); + expect(spy.hasTelemetryKey).toBe(true); + }); +}); + // A Control stream that closes cleanly with no ops — the shortest complete run. async function* emptyControlStream(): AsyncGenerator {} diff --git a/packages/compass-agent/src/cli.ts b/packages/compass-agent/src/cli.ts index a1d8c689b..494d759b1 100644 --- a/packages/compass-agent/src/cli.ts +++ b/packages/compass-agent/src/cli.ts @@ -51,6 +51,10 @@ import { ruleCapability, } from "@oh-my-pi/pi-coding-agent/capability/rule"; import { MCPManager } from "@oh-my-pi/pi-coding-agent/mcp"; +import { + initTelemetryExport, + isTelemetryExportEnabled, +} from "@oh-my-pi/pi-coding-agent/telemetry-export"; import { YAML } from "bun"; import { CompassAgent } from "./agent"; import { CommsBroker, createCommsTools } from "./comms"; @@ -208,6 +212,60 @@ export function deriveLitellmMcpUrl( return `${base}/mcp/`; } +/** + * Whether an OTLP trace endpoint is configured — the pre-registration gate for + * the loop's OpenTelemetry activation (design + * docs/designs/platform/compass-agent-loop-otel/design.md T1). Mirrors the + * predicate `initTelemetryExport` applies before it registers a provider + * (`telemetry-export.ts`): an endpoint present via + * `OTEL_EXPORTER_OTLP_TRACES_ENDPOINT` (falling back to the base + * `OTEL_EXPORTER_OTLP_ENDPOINT`) AND the kill-switches clear + * (`OTEL_SDK_DISABLED` not `true`, `OTEL_TRACES_EXPORTER` not naming `none`). + * + * This is the gate for `main`'s env writes + `init()` call: with no endpoint the + * whole activation is skipped so `process.env` stays UNMUTATED and the session + * build is bit-identical to a no-telemetry container (Global Constraints, "Off + * by default"; F2). The authoritative "did a provider actually register" check + * is `isTelemetryExportEnabled()` AFTER `init()` — which additionally declines + * an unsupported transport protocol — and that is what gates the `telemetry` + * session option. + */ +export function isTelemetryEndpointConfigured( + env: Record, +): boolean { + if (env.OTEL_SDK_DISABLED?.trim().toLowerCase() === "true") return false; + if ( + env.OTEL_TRACES_EXPORTER?.split(",").some( + (entry) => entry.trim().toLowerCase() === "none", + ) + ) { + return false; + } + const endpoint = + env.OTEL_EXPORTER_OTLP_TRACES_ENDPOINT ?? env.OTEL_EXPORTER_OTLP_ENDPOINT; + return endpoint !== undefined && endpoint !== ""; +} + +/** + * The loop-telemetry activation hooks, injectable at the `MainDeps` seam. The + * default binds the reused `@oh-my-pi/pi-coding-agent/telemetry-export` module. + * A test overrides it so it can assert `main`'s gating/env logic WITHOUT the + * real `initTelemetryExport`, which registers a live global TracerProvider + a + * real OTLP exporter with no teardown and would poison every later test in the + * shared process (design record F3). + */ +interface TelemetryHooks { + /** Register the global provider when an endpoint is configured. Idempotent. */ + init: () => Promise; + /** Whether a real provider registered — gates the `telemetry` session option. */ + isEnabled: () => boolean; +} + +const defaultTelemetryHooks: TelemetryHooks = { + init: initTelemetryExport, + isEnabled: isTelemetryExportEnabled, +}; + /** One provider's credential in the seed file. Mirrors the SDK's `ApiKeyCredential`. */ interface SeedEntry { readonly type?: string; @@ -519,6 +577,15 @@ export interface MainDeps { * way to reach the customTools wiring and the teardown-disconnect barrier. */ connectMcp?: (cwd: string, mcp: MountedMcp) => Promise; + /** + * Loop-OpenTelemetry activation hooks (design + * docs/designs/platform/compass-agent-loop-otel/design.md T1). Defaults to the + * reused telemetry-export module. Injectable ONLY so a test asserts the + * gating/env logic without the real `initTelemetryExport` — it registers a + * live global provider + OTLP exporter with no teardown (F3), so it must never + * run in the shared test process. + */ + telemetry?: TelemetryHooks; } /** @@ -755,6 +822,29 @@ export async function main( ).items; const rules = [...mounted.rules, ...discoveredRules]; + // Loop OpenTelemetry activation (design + // docs/designs/platform/compass-agent-loop-otel/design.md T1). Gated HARD on + // an OTLP endpoint being configured: with none set this block is skipped + // whole, so process.env is UNMUTATED and the createAgentSession path below is + // bit-identical to a no-telemetry build (Global Constraints, "Off by default"; + // F2 — an unconditional env write would leak to every tool subprocess). The + // order is load-bearing: the loop provider reads OTEL_SERVICE_NAME / + // OTEL_RESOURCE_ATTRIBUTES at registration time, so the env defaults must be + // in place BEFORE init(). The session id the manager owns (fresh-minted, or + // the resumed header's id after setSessionFile above) is the shared + // cross-signal join key (Decision 3a): APPENDED to OTEL_RESOURCE_ATTRIBUTES, + // never clobbering a deployer-set value. + const telemetryHooks = deps.telemetry ?? defaultTelemetryHooks; + if (isTelemetryEndpointConfigured(process.env)) { + process.env.OTEL_SERVICE_NAME ??= "compass-agent"; + const joinKey = `compass.session.id=${manager.getSessionId()}`; + const existing = process.env.OTEL_RESOURCE_ATTRIBUTES; + process.env.OTEL_RESOURCE_ATTRIBUTES = existing + ? `${existing},${joinKey}` + : joinKey; + await telemetryHooks.init(); + } + const { session } = await (deps.createSession ?? createAgentSession)({ cwd, modelPattern: resolveModelSelector(env), @@ -837,6 +927,12 @@ export async function main( ], } : {}), + // Loop telemetry (design docs/designs/platform/compass-agent-loop-otel/ + // design.md T1): `{}` is the complete activation — the empty config enables + // the loop's GenAI spans (types.ts). Keyed off the AUTHORITATIVE + // post-registration check, so the key is OMITTED entirely when export is + // off and the loop keeps its literal-undefined zero-lookup path. + ...(telemetryHooks.isEnabled() ? { telemetry: {} } : {}), }); // Post-construction assignment, not a `createAgentSession` option: the SDK From 4310216673841f140a378548478787f8abd612ed Mon Sep 17 00:00:00 2001 From: mintaka Date: Wed, 26 Aug 2026 02:55:36 -0400 Subject: [PATCH 2/2] test(compass-agent): pin loop-OTel option gate to post-registration isEnabled (RIG-2508 T1) Review of #648 flagged that the load-bearing "option gate authority" invariant -- the telemetry session key gates on the post-registration isEnabled(), not on the endpoint being configured -- had no behavioral test. A regression swapping the gate to isTelemetryEndpointConfigured would ship green because every existing test conflates endpoint-set with enabled-after-init. Add the distinguishing branch: telemetryDeps now takes registerOnInit (default true); false models the protocol-decline case where init() runs (endpoint gate fired, env defaults written) but no provider registers, so isEnabled() stays false. The new test asserts that with an endpoint set and a declining provider, init ran and env was written yet NO telemetry key is added. Verified non-vacuous: swapping the gate to the endpoint predicate reds this test alone. Co-authored-by: Matt Wilkinson --- packages/compass-agent/src/cli.test.ts | 48 +++++++++++++++++++++++--- 1 file changed, 44 insertions(+), 4 deletions(-) diff --git a/packages/compass-agent/src/cli.test.ts b/packages/compass-agent/src/cli.test.ts index 52ae92eac..339af9bfd 100644 --- a/packages/compass-agent/src/cli.test.ts +++ b/packages/compass-agent/src/cli.test.ts @@ -1646,9 +1646,12 @@ describe("main activates loop OpenTelemetry", () => { }); // A recording telemetry seam + the captured createSession options. `isEnabled` - // mirrors the real module: it reports true only AFTER init() ran (a real - // provider registered), so the option gate keys off registration, not the - // endpoint. The seam records call order so a test can pin env-before-init. + // mirrors the real module: it reports true only AFTER init() ran AND a provider + // actually registered, so the option gate keys off registration, not the + // endpoint. `registerOnInit` (default true) models the ordinary success; set it + // false to model the protocol-decline branch — init() runs (endpoint gate + // fired, env defaults written) yet no provider registers, so isEnabled() stays + // false. The seam records call order so a test can pin env-before-init. interface TelemetrySpy { calls: string[]; telemetryOption: unknown; @@ -1658,6 +1661,7 @@ describe("main activates loop OpenTelemetry", () => { session: FakeSession, transport: RunnerTransport, spy: TelemetrySpy, + registerOnInit = true, ): MainDeps { let registered = false; return { @@ -1676,7 +1680,7 @@ describe("main activates loop OpenTelemetry", () => { spy.calls.push( `init:${process.env.OTEL_SERVICE_NAME}:${process.env.OTEL_RESOURCE_ATTRIBUTES}`, ); - registered = true; + if (registerOnInit) registered = true; return Promise.resolve(); }, isEnabled: () => registered, @@ -1813,6 +1817,42 @@ describe("main activates loop OpenTelemetry", () => { ); expect(spy.hasTelemetryKey).toBe(true); }); + + // Option gate AUTHORITY: the `telemetry` session key gates on the + // post-registration isEnabled() — did a provider actually register — NOT on the + // endpoint being configured (Decision 1 + Global Constraints). The distinguishing + // branch is a set endpoint whose protocol the real module can't honor: init() + // runs (endpoint gate fired, env defaults written) yet declines to register, so + // isEnabled() stays false and NO telemetry key is added. Non-vacuity: swapping + // the gate from `telemetryHooks.isEnabled()` to `isTelemetryEndpointConfigured` + // reds this (endpoint is set ⇒ key would appear) while every other test in the + // block stays green — this is the only test that pins the two gates apart. + test("endpoint set but provider declines to register ⇒ init ran, env written, but NO telemetry key", async () => { + process.env.OTEL_EXPORTER_OTLP_ENDPOINT = "http://collector:4318"; + const spy: TelemetrySpy = { + calls: [], + telemetryOption: "SENTINEL", + hasTelemetryKey: true, + }; + await main( + { HOME: scratch() }, + telemetryDeps( + fakeSession(), + fakeCarrier(emptyLog(), { control: emptyControlStream }), + spy, + false, + ), + ); + // init() DID run — the endpoint gate fired and wrote the env defaults before + // the (declining) registration attempt. + expect(spy.calls).toHaveLength(1); + expect(spy.calls[0]).toMatch(/^init:compass-agent:compass\.session\.id=.+/); + expect(process.env.OTEL_SERVICE_NAME).toBe("compass-agent"); + // But registration declined ⇒ isEnabled() false ⇒ the option gate withholds + // the telemetry key, even though the endpoint is configured. + expect(spy.hasTelemetryKey).toBe(false); + expect(spy.telemetryOption).toBeUndefined(); + }); }); // A Control stream that closes cleanly with no ops — the shortest complete run.