From 6b08c1464a28bb638291e734712444c67b80dcc8 Mon Sep 17 00:00:00 2001 From: anandgupta42 Date: Thu, 15 Oct 2026 16:03:21 -0700 Subject: [PATCH 1/3] feat: add client_connected telemetry and surface/client labels for the IDE funnel Make the serve-process (IDE extension) onboarding funnel measurable. Telemetry showed ~92% of installs run the VS Code extension (source "datamates") as an `altimate serve` backend, and the launch->session drop there was invisible: the serve process emitted `startup_ready` then nothing until `session_start`. - `startup_ready` now carries `surface` and `client` (`ALTIMATE_CLI_CLIENT`, e.g. "datamates") as queryable properties. The surface label was previously only on `command`, and there was no client dimension at all. - New `client_connected` event: fires once per process on the first authenticated subscription to the event stream (`GET /event` / `/global/event`) in `Server.createApp`. Splits "backend launched but no client ever connected" from "connected but never sent a message". Added to `ANCHOR_EVENTS` so it flushes immediately (a short-lived extension host can die before the 5s interval). Telemetry-only, fail-safe and non-blocking. The server.ts hook is wrapped in `altimate_change` markers (upstream-shared file); the telemetry module is altimate-owned under src/altimate/**. Tests: first-run-health.test.ts covers the new fields and once-per-process emit. Co-Authored-By: Claude Opus 4.8 --- .../opencode/src/altimate/telemetry/index.ts | 35 +++++++++++++ packages/opencode/src/server/server.ts | 8 +++ .../telemetry/first-run-health.test.ts | 49 +++++++++++++++++++ .../skill/release-v0.11.1-adversarial.test.ts | 2 + .../test/telemetry/config-opt-out.test.ts | 2 + 5 files changed, 96 insertions(+) diff --git a/packages/opencode/src/altimate/telemetry/index.ts b/packages/opencode/src/altimate/telemetry/index.ts index f350d9783c..afbf869a4a 100644 --- a/packages/opencode/src/altimate/telemetry/index.ts +++ b/packages/opencode/src/altimate/telemetry/index.ts @@ -674,11 +674,26 @@ export namespace Telemetry { session_id: string /** Top-level CLI command, e.g. "tui", "serve", "run". */ command: string + /** Same value as `command`, under the name dashboards query (Properties.surface). */ + surface: string + /** CLI client from ALTIMATE_CLI_CLIENT, e.g. "datamates" for the VS Code extension. */ + client: string /** Wall time from process start until the command could serve its first request or frame. */ duration_ms: number /** True when this process emitted a non-upgrade first_launch, i.e. a brand-new machine. */ fresh_install: boolean } + | { + // First time a client attaches to a running server (once per process). Splits + // "backend launched but no client ever connected" from "connected but never sent a message". + type: "client_connected" + timestamp: number + session_id: string + surface: string + client: string + /** Process uptime when the first client attached. */ + since_start_ms: number + } | { type: "event_loop_stall" timestamp: number @@ -2133,6 +2148,8 @@ export namespace Telemetry { const ANCHOR_EVENTS = new Set([ "first_launch", "startup_ready", + // Once per process, and a short-lived extension host may die before the 5 s flush interval. + "client_connected", "event_loop_stall", "altimate_base_registration", "session_start", @@ -2148,6 +2165,7 @@ export namespace Telemetry { let command = process.env[COMMAND_ENV] ?? "unknown" let freshInstall = false let startupReported = false + let clientConnectedReported = false let loopTimer: ReturnType | undefined let loopExpectedAt = 0 let loopStallsEmitted = 0 @@ -2172,11 +2190,27 @@ export namespace Telemetry { timestamp: Date.now(), session_id: sessionId, command, + surface: command, + client: Flag.ALTIMATE_CLI_CLIENT, duration_ms: Math.round(performance.now()), fresh_install: freshInstall, }) } + /** Emit client_connected once per process; called from the server's event-stream routes. */ + export function clientConnected() { + if (clientConnectedReported) return + clientConnectedReported = true + Telemetry.track({ + type: "client_connected", + timestamp: Date.now(), + session_id: sessionId, + surface: command, + client: Flag.ALTIMATE_CLI_CLIENT, + since_start_ms: Math.round(performance.now()), + }) + } + /** Pure lag check, exported for tests: the stall event when a tick is late by more than the threshold. */ export function loopStallFor( now: number, @@ -2252,6 +2286,7 @@ export namespace Telemetry { command = process.env[COMMAND_ENV] ?? "unknown" freshInstall = false startupReported = false + clientConnectedReported = false loopStallsEmitted = 0 loopExpectedAt = 0 } diff --git a/packages/opencode/src/server/server.ts b/packages/opencode/src/server/server.ts index e5ff81ec39..a210d4ba6a 100644 --- a/packages/opencode/src/server/server.ts +++ b/packages/opencode/src/server/server.ts @@ -312,6 +312,14 @@ export namespace Server { // altimate_change end return basicAuth({ username, password })(c, next) }) + // altimate_change start — telemetry: first authenticated event-stream subscription = a client is really attached + .use(async (c, next) => { + if (c.req.method === "GET" && (c.req.path === "/event" || c.req.path === "/global/event")) { + void import("../altimate/telemetry").then((m) => m.Telemetry.clientConnected()).catch(() => {}) + } + return next() + }) + // altimate_change end .use(async (c, next) => { const skipLogging = c.req.path === "/log" if (!skipLogging) { diff --git a/packages/opencode/test/altimate/telemetry/first-run-health.test.ts b/packages/opencode/test/altimate/telemetry/first-run-health.test.ts index aa91339a7d..a3b8215507 100644 --- a/packages/opencode/test/altimate/telemetry/first-run-health.test.ts +++ b/packages/opencode/test/altimate/telemetry/first-run-health.test.ts @@ -47,6 +47,55 @@ describe("first-run health telemetry", () => { } }) + test("startup_ready carries surface and client as top-level properties", () => { + const events: Telemetry.Event[] = [] + const spy = spyOn(Telemetry, "track").mockImplementation((event) => { + events.push(event) + }) + const saved = process.env.ALTIMATE_CLI_CLIENT + process.env.ALTIMATE_CLI_CLIENT = "datamates" + try { + Telemetry.startupReady("serve") + const event = events.find((e) => e.type === "startup_ready") as Extract< + Telemetry.Event, + { type: "startup_ready" } + > + expect(event.surface).toBe("serve") + expect(event.client).toBe("datamates") + } finally { + if (saved === undefined) delete process.env.ALTIMATE_CLI_CLIENT + else process.env.ALTIMATE_CLI_CLIENT = saved + spy.mockRestore() + } + }) + + test("client_connected is emitted exactly once per process with surface and client", () => { + const events: Telemetry.Event[] = [] + const spy = spyOn(Telemetry, "track").mockImplementation((event) => { + events.push(event) + }) + const saved = process.env.ALTIMATE_CLI_CLIENT + process.env.ALTIMATE_CLI_CLIENT = "datamates" + try { + Telemetry.setCommand("serve") + Telemetry.clientConnected() + Telemetry.clientConnected() + Telemetry.clientConnected() + const connected = events.filter((e) => e.type === "client_connected") as Extract< + Telemetry.Event, + { type: "client_connected" } + >[] + expect(connected).toHaveLength(1) + expect(connected[0].surface).toBe("serve") + expect(connected[0].client).toBe("datamates") + expect(connected[0].since_start_ms).toBeGreaterThan(0) + } finally { + if (saved === undefined) delete process.env.ALTIMATE_CLI_CLIENT + else process.env.ALTIMATE_CLI_CLIENT = saved + spy.mockRestore() + } + }) + test("loopStallFor reports only lags beyond the threshold", () => { expect(Telemetry.loopStallFor(1_000, 900, 500, "main")).toBeUndefined() const stall = Telemetry.loopStallFor(2_000, 500, 1_000, "worker") diff --git a/packages/opencode/test/skill/release-v0.11.1-adversarial.test.ts b/packages/opencode/test/skill/release-v0.11.1-adversarial.test.ts index f334161318..6329315632 100644 --- a/packages/opencode/test/skill/release-v0.11.1-adversarial.test.ts +++ b/packages/opencode/test/skill/release-v0.11.1-adversarial.test.ts @@ -465,6 +465,8 @@ describe("track — anchor events before init", () => { timestamp: Date.now(), session_id: "pre-init", command: "run", + surface: "run", + client: "", duration_ms: 1, fresh_install: false, }) diff --git a/packages/opencode/test/telemetry/config-opt-out.test.ts b/packages/opencode/test/telemetry/config-opt-out.test.ts index e797324931..57fb520d4a 100644 --- a/packages/opencode/test/telemetry/config-opt-out.test.ts +++ b/packages/opencode/test/telemetry/config-opt-out.test.ts @@ -109,6 +109,8 @@ describe("Telemetry.init() inside an Instance context — config.telemetry.disab timestamp: Date.now(), session_id: "config-opt-out-enabled", command: "test", + surface: "test", + client: "", duration_ms: 1, fresh_install: false, }) From a91fbe4ea68c086e2ec45f139cbbfdba1b8eab8d Mon Sep 17 00:00:00 2001 From: anandgupta42 Date: Thu, 15 Oct 2026 20:24:02 -0700 Subject: [PATCH 2/3] refactor: cover client_connected server wiring and drop the redundant client field MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the independent review of the IDE-funnel telemetry change. - Drop the `client` property from `startup_ready` and `client_connected`. `toAppInsightsEnvelopes` already stamps `source: Flag.ALTIMATE_CLI_CLIENT` onto every event that has no `source` of its own, and neither of these events does, so `client` was an exact duplicate of `source` — dashboards could already filter on `source=datamates`. `surface` stays on both events: `client_connected` has no `command`, so `surface` is the one key that joins the two funnel stages, and IDE-funnel reads filter `surface=="serve"`. - Add `test/server/altimate-client-connected-wiring.test.ts`: drives real requests through `Server.createApp` and asserts the middleware calls `Telemetry.clientConnected()` for `GET /event` and `GET /global/event`, and not for other paths or non-GET methods. Nothing previously exercised the server hook that produces the signal — deleting it or changing the path literals left the suite green. Verified non-vacuous by mutation. Tests: first-run-health, config-opt-out and release-v0.11.1-adversarial updated for the dropped field. Package typecheck clean; telemetry suites 394 pass. Co-Authored-By: Claude Opus 4.8 --- .../opencode/src/altimate/telemetry/index.ts | 9 +-- .../telemetry/first-run-health.test.ts | 14 +--- .../altimate-client-connected-wiring.test.ts | 74 +++++++++++++++++++ .../skill/release-v0.11.1-adversarial.test.ts | 1 - .../test/telemetry/config-opt-out.test.ts | 1 - 5 files changed, 79 insertions(+), 20 deletions(-) create mode 100644 packages/opencode/test/server/altimate-client-connected-wiring.test.ts diff --git a/packages/opencode/src/altimate/telemetry/index.ts b/packages/opencode/src/altimate/telemetry/index.ts index afbf869a4a..5c16568c1b 100644 --- a/packages/opencode/src/altimate/telemetry/index.ts +++ b/packages/opencode/src/altimate/telemetry/index.ts @@ -674,10 +674,9 @@ export namespace Telemetry { session_id: string /** Top-level CLI command, e.g. "tui", "serve", "run". */ command: string - /** Same value as `command`, under the name dashboards query (Properties.surface). */ + /** Same value as `command`, under the name dashboards query (Properties.surface). Shared + * with `client_connected`, which has no `command`, so the funnel joins on one key. */ surface: string - /** CLI client from ALTIMATE_CLI_CLIENT, e.g. "datamates" for the VS Code extension. */ - client: string /** Wall time from process start until the command could serve its first request or frame. */ duration_ms: number /** True when this process emitted a non-upgrade first_launch, i.e. a brand-new machine. */ @@ -689,8 +688,8 @@ export namespace Telemetry { type: "client_connected" timestamp: number session_id: string + /** Launching command/surface ("serve", "run", "tui"); IDE-funnel reads filter surface=="serve". */ surface: string - client: string /** Process uptime when the first client attached. */ since_start_ms: number } @@ -2191,7 +2190,6 @@ export namespace Telemetry { session_id: sessionId, command, surface: command, - client: Flag.ALTIMATE_CLI_CLIENT, duration_ms: Math.round(performance.now()), fresh_install: freshInstall, }) @@ -2206,7 +2204,6 @@ export namespace Telemetry { timestamp: Date.now(), session_id: sessionId, surface: command, - client: Flag.ALTIMATE_CLI_CLIENT, since_start_ms: Math.round(performance.now()), }) } diff --git a/packages/opencode/test/altimate/telemetry/first-run-health.test.ts b/packages/opencode/test/altimate/telemetry/first-run-health.test.ts index a3b8215507..c609540480 100644 --- a/packages/opencode/test/altimate/telemetry/first-run-health.test.ts +++ b/packages/opencode/test/altimate/telemetry/first-run-health.test.ts @@ -47,13 +47,11 @@ describe("first-run health telemetry", () => { } }) - test("startup_ready carries surface and client as top-level properties", () => { + test("startup_ready carries surface as a top-level property", () => { const events: Telemetry.Event[] = [] const spy = spyOn(Telemetry, "track").mockImplementation((event) => { events.push(event) }) - const saved = process.env.ALTIMATE_CLI_CLIENT - process.env.ALTIMATE_CLI_CLIENT = "datamates" try { Telemetry.startupReady("serve") const event = events.find((e) => e.type === "startup_ready") as Extract< @@ -61,21 +59,16 @@ describe("first-run health telemetry", () => { { type: "startup_ready" } > expect(event.surface).toBe("serve") - expect(event.client).toBe("datamates") } finally { - if (saved === undefined) delete process.env.ALTIMATE_CLI_CLIENT - else process.env.ALTIMATE_CLI_CLIENT = saved spy.mockRestore() } }) - test("client_connected is emitted exactly once per process with surface and client", () => { + test("client_connected is emitted exactly once per process with surface", () => { const events: Telemetry.Event[] = [] const spy = spyOn(Telemetry, "track").mockImplementation((event) => { events.push(event) }) - const saved = process.env.ALTIMATE_CLI_CLIENT - process.env.ALTIMATE_CLI_CLIENT = "datamates" try { Telemetry.setCommand("serve") Telemetry.clientConnected() @@ -87,11 +80,8 @@ describe("first-run health telemetry", () => { >[] expect(connected).toHaveLength(1) expect(connected[0].surface).toBe("serve") - expect(connected[0].client).toBe("datamates") expect(connected[0].since_start_ms).toBeGreaterThan(0) } finally { - if (saved === undefined) delete process.env.ALTIMATE_CLI_CLIENT - else process.env.ALTIMATE_CLI_CLIENT = saved spy.mockRestore() } }) diff --git a/packages/opencode/test/server/altimate-client-connected-wiring.test.ts b/packages/opencode/test/server/altimate-client-connected-wiring.test.ts new file mode 100644 index 0000000000..dccc0e680d --- /dev/null +++ b/packages/opencode/test/server/altimate-client-connected-wiring.test.ts @@ -0,0 +1,74 @@ +// altimate_change — regression test for the server-side wiring that produces `client_connected`. +// +// The telemetry unit tests (test/altimate/telemetry/first-run-health.test.ts) prove the +// `Telemetry.clientConnected()` latch, but nothing exercised the `.use` middleware in +// `Server.createApp` that actually calls it. Without this test, deleting the middleware, changing +// the `/event` / `/global/event` path literals, dropping the `GET` guard, or moving the hook ahead +// of basicAuth would all leave the suite green while silently killing the IDE-funnel signal. +// +// The middleware fires `void import("../altimate/telemetry").then((m) => m.Telemetry.clientConnected())` +// after next() is set up, so the call lands on a later microtask; each case waits a tick before +// asserting. We spy on `Telemetry.clientConnected` itself (not `track`) so the assertion is purely +// about the middleware deciding to call it — the once-per-process latch lives inside that function +// and is covered elsewhere. +// +// These cases catch deleting the middleware, changing the `/event` / `/global/event` path literals, +// and dropping the `GET` guard. The remaining regression the reviewer named — moving the hook above +// basicAuth so unauthenticated probes count — is not exercised here: auth is enforced by a separate +// `basicAuth` middleware gated on `Flag.OPENCODE_SERVER_PASSWORD`, which is a module-level const +// frozen at load (flag.ts), and CI runs the package in one Bun process, so a password set at test +// time can't drive basicAuth to 401. The ordering is enforced structurally: this hook is registered +// immediately after the basicAuth `.use` in `Server.createApp`. +import { afterEach, beforeEach, describe, expect, spyOn, test } from "bun:test" +import { Server } from "../../src/server/server" +import { Telemetry } from "../../src/altimate/telemetry" +import { resetDatabase } from "./db" +import { disposeAllInstances } from "../fixture/fixture" + +const tick = () => new Promise((r) => setTimeout(r, 15)) + +async function cancelBody(response: Response) { + await response.body?.cancel().catch(() => {}) +} + +let spy: ReturnType> + +beforeEach(() => { + spy = spyOn(Telemetry, "clientConnected").mockImplementation(() => {}) +}) + +afterEach(async () => { + spy.mockRestore() + await disposeAllInstances() + await resetDatabase() +}) + +describe("client_connected server wiring", () => { + test("GET /event invokes Telemetry.clientConnected", async () => { + const response = await Server.Default().request("/event") + await cancelBody(response) + await tick() + expect(spy).toHaveBeenCalled() + }) + + test("GET /global/event invokes Telemetry.clientConnected", async () => { + const response = await Server.Default().request("/global/event") + await cancelBody(response) + await tick() + expect(spy).toHaveBeenCalled() + }) + + test("a non-event path does not invoke Telemetry.clientConnected", async () => { + const response = await Server.Default().request("/config") + await cancelBody(response) + await tick() + expect(spy).not.toHaveBeenCalled() + }) + + test("a non-GET request to /event does not invoke Telemetry.clientConnected", async () => { + const response = await Server.Default().request("/event", { method: "POST" }) + await cancelBody(response) + await tick() + expect(spy).not.toHaveBeenCalled() + }) +}) diff --git a/packages/opencode/test/skill/release-v0.11.1-adversarial.test.ts b/packages/opencode/test/skill/release-v0.11.1-adversarial.test.ts index 6329315632..5a805bdb51 100644 --- a/packages/opencode/test/skill/release-v0.11.1-adversarial.test.ts +++ b/packages/opencode/test/skill/release-v0.11.1-adversarial.test.ts @@ -466,7 +466,6 @@ describe("track — anchor events before init", () => { session_id: "pre-init", command: "run", surface: "run", - client: "", duration_ms: 1, fresh_install: false, }) diff --git a/packages/opencode/test/telemetry/config-opt-out.test.ts b/packages/opencode/test/telemetry/config-opt-out.test.ts index 57fb520d4a..35d0d9495d 100644 --- a/packages/opencode/test/telemetry/config-opt-out.test.ts +++ b/packages/opencode/test/telemetry/config-opt-out.test.ts @@ -110,7 +110,6 @@ describe("Telemetry.init() inside an Instance context — config.telemetry.disab session_id: "config-opt-out-enabled", command: "test", surface: "test", - client: "", duration_ms: 1, fresh_install: false, }) From 45fc7441fdd1bcd9ff18c00ceaee83b5c00a8ea7 Mon Sep 17 00:00:00 2001 From: anandgupta42 Date: Thu, 15 Oct 2026 21:00:06 -0700 Subject: [PATCH 3/3] fix: only count real external clients in client_connected Code review found the serve-process trace consumer trips the metric. On `altimate serve`, `subscribeTraceConsumer` immediately opens an in-process GET /event subscription (trace-consumer.ts), which hit the client_connected middleware and consumed the once-per-process latch before any IDE client attached. The event would fire on ~100% of serve launches and measure the internal subscription, not a real client. - The internal subscription now sets `x-altimate-trace-consumer`; the hook skips requests carrying it. - The hook emits after `next()` and only on a 2xx response, so a request that fails before the event stream is established no longer burns the latch (also addresses the "record after the stream starts" review note). - Wiring test covers the trace-consumer skip and uses a bounded poll instead of a fixed sleep (removes the flagged timing race). Tests: telemetry, server-wiring and trace-consumer suites pass; typecheck clean. Co-Authored-By: Claude Opus 4.8 --- .../altimate/observability/trace-consumer.ts | 3 + packages/opencode/src/server/server.ts | 15 +++- .../altimate-client-connected-wiring.test.ts | 83 ++++++++++++------- 3 files changed, 68 insertions(+), 33 deletions(-) diff --git a/packages/opencode/src/altimate/observability/trace-consumer.ts b/packages/opencode/src/altimate/observability/trace-consumer.ts index ae5ce06c46..fa852d0da5 100644 --- a/packages/opencode/src/altimate/observability/trace-consumer.ts +++ b/packages/opencode/src/altimate/observability/trace-consumer.ts @@ -388,6 +388,9 @@ export function subscribeTraceConsumer( (() => { const fetchFn = (async (fetchInput: RequestInfo | URL, init?: RequestInit) => { const request = new Request(fetchInput, init) + // Mark this as the in-process observability subscription so the client_connected telemetry + // hook in Server.createApp does not count it as a real external client attaching. + request.headers.set("x-altimate-trace-consumer", "1") const password = Flag.OPENCODE_SERVER_PASSWORD if (password) { const username = Flag.OPENCODE_SERVER_USERNAME ?? "opencode" diff --git a/packages/opencode/src/server/server.ts b/packages/opencode/src/server/server.ts index a210d4ba6a..717a53d939 100644 --- a/packages/opencode/src/server/server.ts +++ b/packages/opencode/src/server/server.ts @@ -312,12 +312,21 @@ export namespace Server { // altimate_change end return basicAuth({ username, password })(c, next) }) - // altimate_change start — telemetry: first authenticated event-stream subscription = a client is really attached + // altimate_change start — telemetry: a real external client attached to the event stream. + // Emit after next() and only on a 2xx, so a request that fails before the stream is + // established doesn't burn the once-per-process latch. Skip the in-process trace consumer + // (serve.ts subscribes to /event for observability via x-altimate-trace-consumer): it is not + // a real client and would otherwise trip the latch on every `serve` launch before the IDE + // attaches. .use(async (c, next) => { - if (c.req.method === "GET" && (c.req.path === "/event" || c.req.path === "/global/event")) { + const isClientEventStream = + c.req.method === "GET" && + (c.req.path === "/event" || c.req.path === "/global/event") && + !c.req.header("x-altimate-trace-consumer") + await next() + if (isClientEventStream && c.res.ok) { void import("../altimate/telemetry").then((m) => m.Telemetry.clientConnected()).catch(() => {}) } - return next() }) // altimate_change end .use(async (c, next) => { diff --git a/packages/opencode/test/server/altimate-client-connected-wiring.test.ts b/packages/opencode/test/server/altimate-client-connected-wiring.test.ts index dccc0e680d..01d7d913de 100644 --- a/packages/opencode/test/server/altimate-client-connected-wiring.test.ts +++ b/packages/opencode/test/server/altimate-client-connected-wiring.test.ts @@ -1,36 +1,48 @@ // altimate_change — regression test for the server-side wiring that produces `client_connected`. // // The telemetry unit tests (test/altimate/telemetry/first-run-health.test.ts) prove the -// `Telemetry.clientConnected()` latch, but nothing exercised the `.use` middleware in -// `Server.createApp` that actually calls it. Without this test, deleting the middleware, changing -// the `/event` / `/global/event` path literals, dropping the `GET` guard, or moving the hook ahead -// of basicAuth would all leave the suite green while silently killing the IDE-funnel signal. +// `Telemetry.clientConnected()` latch; this file proves the `.use` middleware in `Server.createApp` +// that calls it. Without it, deleting the middleware, changing the `/event` / `/global/event` path +// literals, dropping the `GET` guard, failing to skip the in-process trace consumer, or emitting on +// a non-2xx would all leave the suite green while breaking (or corrupting) the IDE-funnel signal. // -// The middleware fires `void import("../altimate/telemetry").then((m) => m.Telemetry.clientConnected())` -// after next() is set up, so the call lands on a later microtask; each case waits a tick before -// asserting. We spy on `Telemetry.clientConnected` itself (not `track`) so the assertion is purely -// about the middleware deciding to call it — the once-per-process latch lives inside that function -// and is covered elsewhere. +// The middleware emits AFTER next() and only when the event route returned a 2xx, via +// `void import("../altimate/telemetry").then((m) => m.Telemetry.clientConnected())`. The call lands +// on a later microtask, so positive cases poll with a bounded timeout rather than sleeping a fixed +// amount. We spy on `Telemetry.clientConnected` itself so the assertion is purely about the +// middleware deciding to call it; the once-per-process latch lives inside that function and is +// covered elsewhere. // -// These cases catch deleting the middleware, changing the `/event` / `/global/event` path literals, -// and dropping the `GET` guard. The remaining regression the reviewer named — moving the hook above -// basicAuth so unauthenticated probes count — is not exercised here: auth is enforced by a separate -// `basicAuth` middleware gated on `Flag.OPENCODE_SERVER_PASSWORD`, which is a module-level const -// frozen at load (flag.ts), and CI runs the package in one Bun process, so a password set at test -// time can't drive basicAuth to 401. The ordering is enforced structurally: this hook is registered -// immediately after the basicAuth `.use` in `Server.createApp`. +// Not exercised here: the "hook moved above basic-auth" case. Auth is a separate `basicAuth` +// middleware gated on `Flag.OPENCODE_SERVER_PASSWORD`, a module-level const frozen at load, and CI +// runs the package in one Bun process, so a password set at test time can't drive a 401. The order +// is enforced structurally: the hook is registered immediately after the basic-auth `.use`, and it +// now only counts a 2xx response, so an auth rejection (non-2xx) cannot trip it. import { afterEach, beforeEach, describe, expect, spyOn, test } from "bun:test" import { Server } from "../../src/server/server" import { Telemetry } from "../../src/altimate/telemetry" import { resetDatabase } from "./db" -import { disposeAllInstances } from "../fixture/fixture" - -const tick = () => new Promise((r) => setTimeout(r, 15)) +import { disposeAllInstances, tmpdir } from "../fixture/fixture" async function cancelBody(response: Response) { await response.body?.cancel().catch(() => {}) } +// The emit is async (dynamic import + .then). Poll for it instead of sleeping a fixed amount. +async function waitForCall(spy: { mock: { calls: unknown[] } }, timeoutMs = 1000): Promise { + const start = Date.now() + while (Date.now() - start < timeoutMs) { + if (spy.mock.calls.length > 0) return true + await new Promise((r) => setTimeout(r, 5)) + } + return false +} + +// A negative can't be proven instantly; give the microtask/import a real chance to fire first. +async function settle() { + await new Promise((r) => setTimeout(r, 50)) +} + let spy: ReturnType> beforeEach(() => { @@ -44,31 +56,42 @@ afterEach(async () => { }) describe("client_connected server wiring", () => { - test("GET /event invokes Telemetry.clientConnected", async () => { - const response = await Server.Default().request("/event") + test("GET /global/event (2xx) invokes Telemetry.clientConnected", async () => { + const response = await Server.Default().request("/global/event") + expect(response.ok).toBe(true) await cancelBody(response) - await tick() - expect(spy).toHaveBeenCalled() + expect(await waitForCall(spy)).toBe(true) }) - test("GET /global/event invokes Telemetry.clientConnected", async () => { - const response = await Server.Default().request("/global/event") + test("GET /event (2xx, resolved instance) invokes Telemetry.clientConnected", async () => { + await using tmp = await tmpdir({ git: true, config: { formatter: false, lsp: false } }) + const response = await Server.Default().request("/event", { headers: { "x-opencode-directory": tmp.path } }) + expect(response.ok).toBe(true) await cancelBody(response) - await tick() - expect(spy).toHaveBeenCalled() + expect(await waitForCall(spy)).toBe(true) + }) + + test("the in-process trace consumer (x-altimate-trace-consumer) is NOT counted", async () => { + const response = await Server.Default().request("/global/event", { + headers: { "x-altimate-trace-consumer": "1" }, + }) + expect(response.ok).toBe(true) + await cancelBody(response) + await settle() + expect(spy).not.toHaveBeenCalled() }) test("a non-event path does not invoke Telemetry.clientConnected", async () => { - const response = await Server.Default().request("/config") + const response = await Server.Default().request("/global/config") await cancelBody(response) - await tick() + await settle() expect(spy).not.toHaveBeenCalled() }) test("a non-GET request to /event does not invoke Telemetry.clientConnected", async () => { const response = await Server.Default().request("/event", { method: "POST" }) await cancelBody(response) - await tick() + await settle() expect(spy).not.toHaveBeenCalled() }) })