diff --git a/packages/loopover-contract/src/api-schemas.ts b/packages/loopover-contract/src/api-schemas.ts index 0b057b8e3..931a6ce32 100644 --- a/packages/loopover-contract/src/api-schemas.ts +++ b/packages/loopover-contract/src/api-schemas.ts @@ -433,6 +433,7 @@ export const RepositorySettingsSchema = z autoProjectMilestoneMatchBackend: z.enum(["github", "linear"]).optional(), gatePack: z.enum(["gittensor", "oss-anti-slop"]), linkedIssueGateMode: z.enum(["off", "advisory", "block"]), + linkedIssueMaintainerExempt: z.boolean().nullable().optional(), duplicatePrGateMode: z.enum(["off", "advisory", "block"]), qualityGateMode: z.enum(["off", "advisory", "block"]), qualityGateMinScore: z.number().nullable().optional(), diff --git a/packages/loopover-engine/src/index.ts b/packages/loopover-engine/src/index.ts index fcb802c00..842434fbf 100644 --- a/packages/loopover-engine/src/index.ts +++ b/packages/loopover-engine/src/index.ts @@ -392,12 +392,21 @@ export { createFakeCodingAgentDriverForFactory, isConfiguredCodingAgentDriver, resolveConfiguredCodingAgentDriverNames, + resolveCodingAgentTelemetryModel, resolveFirstConfiguredCodingAgentDriverName, runCodingAgentAttempt, type CodingAgentDriverName, type CreateCodingAgentDriverOptions, type RunCodingAgentAttemptOptions, } from "./miner/driver-factory.js"; +export { + emitMinerAiGeneration, + hasMinerAiGenerationSink, + setMinerAiGenerationSink, + withCodingAgentGenerationCapture, + type MinerAiGenerationRecord, + type MinerAiGenerationSink, +} from "./miner/ai-generation-sink.js"; export * from "./miner/attempt-metering.js"; export { buildRepoMap, @@ -415,6 +424,7 @@ export { } from "./miner/repo-map.js"; export { createAgentSdkCodingAgentDriver, + readAgentSdkResultUsage, type AgentSdkHooks, type AgentSdkQueryFn, type AgentSdkQueryOptions, @@ -423,6 +433,7 @@ export { export { buildChatPrompt, CHAT_GROUNDING_MCP_SERVER_NAME, + CHAT_GROUNDING_PROVIDER, CHAT_GROUNDING_TOOL_NAMES, CHAT_REDACTED_TEXT, CHAT_SYSTEM_PROMPT, diff --git a/packages/loopover-engine/src/miner/agent-sdk-driver.ts b/packages/loopover-engine/src/miner/agent-sdk-driver.ts index 6d20127b9..c42a5b5b5 100644 --- a/packages/loopover-engine/src/miner/agent-sdk-driver.ts +++ b/packages/loopover-engine/src/miner/agent-sdk-driver.ts @@ -111,6 +111,25 @@ function tokensFromResultMessage(resultMessage: Record | null): }; } +/** The billing facts an SDK `result` frame carries: the token split above plus the session's real dollar cost. + * `SDKResultSuccess`/`SDKResultError` both declare `total_cost_usd: number` unconditionally — present whenever + * a result message arrived at all, success or not (the session was billed either way), absent only when the + * stream produced no result frame. `finiteNonNegativeNumber` (not a bare `typeof`) because a malformed value + * from an untyped source must degrade to undefined rather than propagate (#5827). + * + * Exported (#10200) so `chat-grounding.ts` reads the SAME frame the same way. It drives its own `query()` + * session and needs the identical usage/cost facts; a private copy there would be a third instance of these + * defensive reads, which is the duplicated-block class #10170 catalogues. */ +export function readAgentSdkResultUsage(resultMessage: Record | null): { + tokens: CodingAgentTokenUsage; + costUsd: number | undefined; +} { + return { + tokens: tokensFromResultMessage(resultMessage), + costUsd: finiteNonNegativeNumber(resultMessage?.total_cost_usd), + }; +} + async function listWorktreeChangedFiles(cwd: string): Promise { const [tracked, untracked] = await Promise.all([ execFileAsync("git", ["-C", cwd, "diff", "--name-only", "HEAD", "--"]), @@ -204,11 +223,7 @@ export function createAgentSdkCodingAgentDriver( // negative) must degrade to undefined here, or it reaches accumulateAttemptUsage unguarded and throws a // RangeError that rejects runIterateLoopCore before any decision is logged (#5827). const turnsUsed = finiteNonNegativeNumber(resultMessage?.num_turns); - // Real dollar cost: the SDK's own SDKResultSuccess/SDKResultError message types both declare - // `total_cost_usd: number` unconditionally -- present whenever a result message arrived at all, success - // or not (the session was billed either way), absent only when the stream produced no result message. - const costUsd = finiteNonNegativeNumber(resultMessage?.total_cost_usd); - const tokenUsage = tokensFromResultMessage(resultMessage); + const { tokens: tokenUsage, costUsd } = readAgentSdkResultUsage(resultMessage); const resultText = typeof resultMessage?.result === "string" ? redactSecrets(resultMessage.result) : ""; const transcript = redactSecrets( diff --git a/packages/loopover-engine/src/miner/ai-generation-sink.ts b/packages/loopover-engine/src/miner/ai-generation-sink.ts new file mode 100644 index 000000000..6644327ab --- /dev/null +++ b/packages/loopover-engine/src/miner/ai-generation-sink.ts @@ -0,0 +1,114 @@ +// Host-registered `$ai_generation` sink for the miner's AMS surfaces (#10200, epic #8286 Phase 3). +// +// Every real model call this package drives -- a coding-agent driver attempt (driver-factory.ts) and a +// chat-grounding session (chat-grounding.ts) -- is spend that belongs in PostHog. The engine cannot capture it +// itself: it is the portable package, and `posthog-node` (or any other vendor client) is exactly the dependency +// it must not carry -- the same boundary cli-subprocess-driver.ts's and coding-agent-construction.ts's own +// headers already document. +// +// The previous shape solved that by attaching the capture at a HOST construction site instead +// (`withCodingAgentAiGenerationCapture`, applied inside `constructProductionCodingAgentDriver`), which made +// opting out silent: `runCodingAgentAttempt` builds its driver through `createCodingAgentDriver` DIRECTLY and +// never passes through that site, so every attempt it ran was uncaptured and nothing anywhere said so. A +// wrapper attached at one of N construction sites is precisely the "two or more places that must agree, with +// nothing enforcing it" shape #10170 catalogues, and #10127's fix for the same class is the precedent: make the +// bypass unrepresentable rather than documenting it. +// +// So the sink is registered ONCE by the host process and consumed inside the engine at the single chokepoint +// every real driver is constructed through. A future third construction site cannot silently opt out, because +// there is nothing left for it to forget to attach. + +import type { CodingAgentDriver } from "./coding-agent-driver.js"; + +/** + * One completed model call, in the host-neutral shape a `$ai_generation` capture needs. Metadata only: model and + * provider ids, timing, token/cost accounting, and the raw caught value on the error path -- there is + * deliberately no field for the prompt, the transcript, or any tool output, matching the ORB side's identical + * policy (`PostHogAiGenerationEvent`, src/selfhost/posthog.ts). + * + * Every token/cost field is OPTIONAL and stays absent when the provider reported nothing (#10207): a fabricated + * 0 is indistinguishable from a real 0 in an aggregate, so the sink must be able to tell "zero" from "unknown". + */ +export type MinerAiGenerationRecord = { + provider: string; + model: string; + latencyMs: number; + isError: boolean; + totalTokens?: number | undefined; + inputTokens?: number | undefined; + outputTokens?: number | undefined; + totalCostUsd?: number | undefined; + error?: unknown; +}; + +/** The host's capture function. Synchronous and fire-and-forget -- a sink that needs to do I/O queues it. */ +export type MinerAiGenerationSink = (record: MinerAiGenerationRecord) => void; + +let sink: MinerAiGenerationSink | undefined; + +/** + * Register (or, with `undefined`, clear) the process-wide sink. Called once by the host during startup -- + * `initMinerPostHog` (packages/loopover-miner/lib/posthog.ts) registers its own capture here when the operator + * has opted in, and registers nothing when they have not, so the no-phone-home default (#6011) is preserved by + * construction: with no sink registered, {@link emitMinerAiGeneration} is a no-op. + * + * Module-level rather than threaded through every call: the same shape posthog.ts's own `client`/`active` module + * state already uses on the host side, and for the same reason -- a process has exactly one telemetry sink, and + * an optional parameter threaded through N call sites is the opt-out this module exists to remove. + */ +export function setMinerAiGenerationSink(next: MinerAiGenerationSink | undefined): void { + sink = next; +} + +/** True when a host sink is registered. Exported for the host's own wiring assertions, not for gating a call. */ +export function hasMinerAiGenerationSink(): boolean { + return sink !== undefined; +} + +/** Report one completed model call. No-op when no sink is registered, and never throws -- telemetry must never + * crash the AI call it is instrumenting, the same contract every capture function in the miner's posthog.ts + * already holds on its own side. */ +export function emitMinerAiGeneration(record: MinerAiGenerationRecord): void { + if (!sink) return; + try { + sink(record); + } catch { + /* A sink that throws is a telemetry bug, never the caller's problem. */ + } +} + +/** + * Wrap a real `CodingAgentDriver` so every attempt it runs reports a generation. Moved here from the miner's own + * construction site (#10200) so the single engine-side factory can apply it to every driver it builds. + * + * `CodingAgentDriverResult` carries the blended `tokensUsed` plus the input/output split when the provider + * reported one (#10198); all of it is forwarded verbatim, and a driver that knows no split simply leaves those + * fields absent rather than having one fabricated. A driver failure is reported via `result.ok === false` (the + * real, observed contract every shipped driver follows -- none of them throw for an ordinary task failure), with + * a genuine thrown exception handled defensively on top. + */ +export function withCodingAgentGenerationCapture(provider: string, model: string, driver: CodingAgentDriver): CodingAgentDriver { + return { + async run(task) { + const startedAtMs = Date.now(); + try { + const result = await driver.run(task); + emitMinerAiGeneration({ + provider, + model, + latencyMs: Date.now() - startedAtMs, + isError: !result.ok, + totalTokens: result.tokensUsed, + inputTokens: result.inputTokens, + outputTokens: result.outputTokens, + totalCostUsd: result.costUsd, + error: result.ok ? undefined : result.error, + }); + return result; + } catch (error) { + emitMinerAiGeneration({ provider, model, latencyMs: Date.now() - startedAtMs, isError: true, error }); + throw error; + } + }, + }; +} diff --git a/packages/loopover-engine/src/miner/chat-grounding.ts b/packages/loopover-engine/src/miner/chat-grounding.ts index 7a22ffefb..c89dfac2b 100644 --- a/packages/loopover-engine/src/miner/chat-grounding.ts +++ b/packages/loopover-engine/src/miner/chat-grounding.ts @@ -18,7 +18,9 @@ // The endpoint is stateless: the caller supplies the full message history per request (no conversation store). import { PUBLIC_FIELD_BLOCKLIST } from "../track-record-summary.js"; -import { resolveFirstConfiguredCodingAgentDriverName } from "./driver-factory.js"; +import { readAgentSdkResultUsage } from "./agent-sdk-driver.js"; +import { emitMinerAiGeneration } from "./ai-generation-sink.js"; +import { resolveCodingAgentTelemetryModel, resolveFirstConfiguredCodingAgentDriverName } from "./driver-factory.js"; /** * The exact read-only tools this endpoint may call — one `server.registerTool(...)` call each in @@ -42,6 +44,11 @@ export const CHAT_GROUNDING_TOOL_NAMES = Object.freeze([ /** The MCP server name the session registers the miner tools under. */ export const CHAT_GROUNDING_MCP_SERVER_NAME = "loopover-miner"; +/** The only provider this endpoint can run on (boundary 1 above) — and therefore the provider id its + * `$ai_generation` telemetry reports (#10200). One definition, consumed by both the fail-closed check and the + * capture, so the two can never disagree about what is actually running. */ +export const CHAT_GROUNDING_PROVIDER = "agent-sdk"; + /** Ceiling on a single conversational session's tool-calling turns. */ const CHAT_MAX_TURNS = 12; @@ -201,7 +208,7 @@ export function resolveChatProviderError( "No coding-agent provider is configured. Chat requires the agent-sdk provider — set MINER_CODING_AGENT_PROVIDER=agent-sdk.", }; } - if (provider !== "agent-sdk") { + if (provider !== CHAT_GROUNDING_PROVIDER) { return { code: "chat_requires_agent_sdk_provider", message: `Chat requires the agent-sdk provider; the configured provider is ${provider}, which is a single-turn, buffered coding driver.`, @@ -254,9 +261,35 @@ function* foldToolResultMessage(message: Record): Generator`), and the telemetry must not call either of them a success. + */ +function chatResultFailureReason(resultMessage: Record | null): string | undefined { + if (!resultMessage) return "chat_grounding_no_result"; + if (resultMessage.is_error === true) return "chat_grounding_errored"; + if (resultMessage.subtype !== "success") { + return `chat_grounding_${typeof resultMessage.subtype === "string" ? resultMessage.subtype : "unknown"}`; + } + return undefined; +} + /** * Drives one grounded conversational turn, yielding wire events. Never throws: an SDK failure becomes an `error` * event, and `done` always terminates the stream — including on the fail-closed provider paths. + * + * #10200: this drives a real `query()` session with a real turn budget, so it is real spend and reports an + * `$ai_generation` through the host sink (ai-generation-sink.ts) on both the completed and the thrown path. + * That also means folding the SDK's `result` frame, which this module previously discarded — it read only the + * `assistant`/`user` messages it turns into wire events, so the session's own usage and cost were on the wire + * and thrown away. + * + * The fail-closed provider path above deliberately emits NOTHING: no model was ever reached, so an + * `$ai_generation` there would fabricate a generation that did not happen. That case is a request which produced + * no generation — the shape the ORB side gives its own separate `selfhost_ai_degraded` event (#10186), which the + * miner has no counterpart for yet. */ export async function* runChatGrounding( messages: ChatMessage[], @@ -272,6 +305,9 @@ export async function* runChatGrounding( const query = resolveChatQuery(options); const mcpServer = options.mcpServer ?? DEFAULT_MCP_SERVER; + const model = resolveCodingAgentTelemetryModel(CHAT_GROUNDING_PROVIDER, env); + const startedAtMs = Date.now(); + let resultMessage: Record | null = null; try { const stream = query({ prompt: buildChatPrompt(messages), @@ -289,9 +325,33 @@ export async function* runChatGrounding( } if (message.type === "user") { yield* foldToolResultMessage(message); + continue; } + // Kept, not re-emitted: the result frame carries usage/cost, never conversational content, so it feeds + // the capture below and never becomes a wire event. + if (message.type === "result") resultMessage = message; } + const { tokens, costUsd } = readAgentSdkResultUsage(resultMessage); + const failure = chatResultFailureReason(resultMessage); + emitMinerAiGeneration({ + provider: CHAT_GROUNDING_PROVIDER, + model, + latencyMs: Date.now() - startedAtMs, + isError: failure !== undefined, + totalTokens: tokens.tokensUsed, + inputTokens: tokens.inputTokens, + outputTokens: tokens.outputTokens, + totalCostUsd: costUsd, + ...(failure === undefined ? {} : { error: new Error(failure) }), + }); } catch (error) { + emitMinerAiGeneration({ + provider: CHAT_GROUNDING_PROVIDER, + model, + latencyMs: Date.now() - startedAtMs, + isError: true, + error, + }); yield { type: "error", code: "chat_grounding_failed", diff --git a/packages/loopover-engine/src/miner/driver-factory.ts b/packages/loopover-engine/src/miner/driver-factory.ts index 3815ed99c..aad136651 100644 --- a/packages/loopover-engine/src/miner/driver-factory.ts +++ b/packages/loopover-engine/src/miner/driver-factory.ts @@ -29,6 +29,7 @@ import { type AgentSdkHooks, type AgentSdkQueryFn, } from "./agent-sdk-driver.js"; +import { withCodingAgentGenerationCapture } from "./ai-generation-sink.js"; /** Provider names the factory resolves: the two concrete drivers from #4266/#4267 (`claude-cli`/`codex-cli` * spawn the respective CLI; `agent-sdk` runs in-process via the Agent SDK) plus the `noop` stub. All are @@ -200,17 +201,27 @@ function createCliProvider( }); } -/** Resolve a concrete driver for `providerName`. Throws on unknown/unconfigured providers (fail-closed). */ -export function createCodingAgentDriver(options: CreateCodingAgentDriverOptions): CodingAgentDriver { - if (options.driver) return options.driver; - const name = options.providerName.trim().toLowerCase(); - const env = options.env ?? {}; - if (!isConfiguredCodingAgentDriver(name, env)) { - throw new Error(`unconfigured_coding_agent_driver:${name}`); - } +/** The model id telemetry should report for `name`: the provider's configured model env var when it declares + * one, else the provider name itself — `agent-sdk` declares none (its session uses the account/CLI default), + * mirroring ai.ts's own "unconfigured → a sensible default" convention. + * + * Moved here from the miner's construction site (#10200) so every construction path names the model + * identically instead of each one re-deriving it. Uses `firstConfiguredEnvValue`, the same reader + * `createCliProvider` already applies to the same env var, so a whitespace-only value resolves to the provider + * name rather than being passed through as a blank model id. */ +export function resolveCodingAgentTelemetryModel(name: string, env: Record): string { + const modelEnvKey = CODING_AGENT_DRIVER_CONFIG_ENV[name as CodingAgentDriverName]?.model; + return (modelEnvKey ? firstConfiguredEnvValue(env[modelEnvKey]) : undefined) ?? name; +} + +/** The provider switch itself. Split out of {@link createCodingAgentDriver} so the capture wrapper below has + * exactly one expression to wrap — a provider arm cannot return around it. */ +function createProviderDriver( + name: string, + options: CreateCodingAgentDriverOptions, + env: Record, +): CodingAgentDriver { switch (name) { - case "noop": - return createNoopCodingAgentDriver(); case "claude-cli": return createCliProvider("claude", "MINER_CODING_AGENT_CLAUDE_MODEL", options, env); case "codex-cli": @@ -228,6 +239,34 @@ export function createCodingAgentDriver(options: CreateCodingAgentDriverOptions) } } +/** Resolve a concrete driver for `providerName`. Throws on unknown/unconfigured providers (fail-closed). + * + * #10200: this is the ONE place a real coding-agent driver is constructed, so it is where `$ai_generation` + * capture is attached — both `constructProductionCodingAgentDriver` (the miner CLI) and + * `resolveDriverForAttempt` (`runCodingAgentAttempt`, below) reach a provider through here, and the second one + * previously produced uncaptured attempts because the wrapper lived at only the first. The capture itself is + * host-supplied (see ai-generation-sink.ts); with no host sink registered it is a no-op. */ +export function createCodingAgentDriver(options: CreateCodingAgentDriverOptions): CodingAgentDriver { + // Test seam: an injected driver is returned verbatim and uncaptured. It never reaches a model, so wrapping it + // would report generations that did not happen — the same reasoning the `noop` arm below rests on. + if (options.driver) return options.driver; + const name = options.providerName.trim().toLowerCase(); + const env = options.env ?? {}; + if (!isConfiguredCodingAgentDriver(name, env)) { + throw new Error(`unconfigured_coding_agent_driver:${name}`); + } + // `noop` is a stub that makes no model call, so it has no generation to report. Capturing it would fabricate + // an $ai_generation for an attempt that never reached a provider — #10207's never-fabricate rule applied to + // the event itself rather than to its token fields. (`resolveDriverForAttempt` reaches the same conclusion + // independently for dry-run/paused attempts, which bypass this factory entirely.) + if (name === "noop") return createNoopCodingAgentDriver(); + return withCodingAgentGenerationCapture( + name, + resolveCodingAgentTelemetryModel(name, env), + createProviderDriver(name, options, env), + ); +} + export type RunCodingAgentAttemptOptions = { providerName: string; env?: Record | undefined; diff --git a/packages/loopover-engine/test/ai-generation-sink.test.ts b/packages/loopover-engine/test/ai-generation-sink.test.ts new file mode 100644 index 000000000..27f1d970e --- /dev/null +++ b/packages/loopover-engine/test/ai-generation-sink.test.ts @@ -0,0 +1,169 @@ +import { test, afterEach } from "node:test"; +import assert from "node:assert/strict"; +import { + createCodingAgentDriver, + emitMinerAiGeneration, + hasMinerAiGenerationSink, + resolveCodingAgentTelemetryModel, + runChatGrounding, + runCodingAgentAttempt, + setMinerAiGenerationSink, + withCodingAgentGenerationCapture, + CHAT_GROUNDING_PROVIDER, + type ChatGroundingEvent, + type ChatQueryFn, + type CodingAgentDriverTask, + type MinerAiGenerationRecord, +} from "../dist/index.js"; + +// Engine-side behavior suite for the host-registered $ai_generation sink (#10200). Mirrored by +// test/unit/miner-ai-generation-sink.test.ts in the app's vitest run; both are needed because the two coverage +// uploads are unioned per line and only this one actually behavior-tests the engine package. + +const task: CodingAgentDriverTask = { + attemptId: "attempt-sink-1", + workingDirectory: "/tmp/worktrees/attempt-sink-1", + acceptanceCriteriaPath: "/tmp/worktrees/attempt-sink-1/ACCEPTANCE-CRITERIA.md", + instructions: "Apply the fix described in ACCEPTANCE-CRITERIA.md.", + maxTurns: 4, +}; + +function recordingSink(): MinerAiGenerationRecord[] { + const records: MinerAiGenerationRecord[] = []; + setMinerAiGenerationSink((record) => records.push(record)); + return records; +} + +afterEach(() => { + setMinerAiGenerationSink(undefined); +}); + +test("emitMinerAiGeneration is a no-op with no sink, and forwards once one is registered", () => { + assert.equal(hasMinerAiGenerationSink(), false); + emitMinerAiGeneration({ provider: "agent-sdk", model: "agent-sdk", latencyMs: 1, isError: false }); + const records = recordingSink(); + assert.equal(hasMinerAiGenerationSink(), true); + emitMinerAiGeneration({ provider: "claude-cli", model: "claude-sonnet-5", latencyMs: 7, isError: false }); + assert.equal(records.length, 1); + setMinerAiGenerationSink(undefined); + emitMinerAiGeneration({ provider: "claude-cli", model: "claude-sonnet-5", latencyMs: 7, isError: false }); + assert.equal(records.length, 1); +}); + +test("a throwing sink never reaches the caller it is instrumenting", () => { + setMinerAiGenerationSink(() => { + throw new Error("posthog exploded"); + }); + assert.doesNotThrow(() => emitMinerAiGeneration({ provider: "agent-sdk", model: "agent-sdk", latencyMs: 1, isError: true })); +}); + +test("resolveCodingAgentTelemetryModel reads the configured model env var, else the provider name", () => { + assert.equal(resolveCodingAgentTelemetryModel("claude-cli", { MINER_CODING_AGENT_CLAUDE_MODEL: "claude-opus-5" }), "claude-opus-5"); + assert.equal(resolveCodingAgentTelemetryModel("agent-sdk", {}), "agent-sdk"); + assert.equal(resolveCodingAgentTelemetryModel("codex-cli", { MINER_CODING_AGENT_CODEX_MODEL: " " }), "codex-cli"); +}); + +test("REGRESSION: runCodingAgentAttempt's own driver path is captured (#10200's named bypass)", async () => { + const records = recordingSink(); + await runCodingAgentAttempt({ + providerName: "claude-cli", + env: { MINER_CODING_AGENT_MODE: "live", MINER_CODING_AGENT_CLAUDE_MODEL: "claude-opus-5" }, + task, + spawn: async () => ({ stdout: "done", code: 0 }), + }); + assert.equal(records.length, 1); + assert.equal(records[0]?.provider, "claude-cli"); + assert.equal(records[0]?.model, "claude-opus-5"); + assert.equal(records[0]?.isError, false); +}); + +test("noop and injected drivers are NOT captured -- neither reaches a model", async () => { + const records = recordingSink(); + await createCodingAgentDriver({ providerName: "noop", env: {} }).run(task); + const injected = { + run: async (_task: CodingAgentDriverTask) => ({ ok: true as const, changedFiles: [], summary: "injected", transcript: "" }), + }; + assert.equal(createCodingAgentDriver({ providerName: "claude-cli", driver: injected }), injected); + await injected.run(task); + assert.equal(records.length, 0); +}); + +test("withCodingAgentGenerationCapture omits unreported usage and carries the driver's own error", async () => { + const records = recordingSink(); + await withCodingAgentGenerationCapture("codex-cli", "gpt-5-codex", { + run: async () => ({ ok: true as const, changedFiles: [], summary: "done", transcript: "" }), + }).run(task); + assert.equal(records[0]?.inputTokens, undefined); + assert.equal(records[0]?.outputTokens, undefined); + assert.equal(records[0]?.totalCostUsd, undefined); + + await withCodingAgentGenerationCapture("codex-cli", "gpt-5-codex", { + run: async () => ({ ok: false as const, changedFiles: [], summary: "failed", transcript: "", error: "codex_timeout_120000ms" }), + }).run(task); + assert.equal(records[1]?.isError, true); + assert.equal(records[1]?.error, "codex_timeout_120000ms"); +}); + +test("withCodingAgentGenerationCapture captures and rethrows a driver exception", async () => { + const records = recordingSink(); + const driver = withCodingAgentGenerationCapture("agent-sdk", "agent-sdk", { + run: async () => { + throw new Error("sdk crashed"); + }, + }); + await assert.rejects(() => driver.run(task), /sdk crashed/); + assert.equal(records[0]?.isError, true); +}); + +const USER_ONLY = [{ role: "user" as const, content: "what is my run state?" }]; + +function queryYielding(messages: Array>): ChatQueryFn { + return () => + (async function* () { + yield* messages; + })(); +} + +async function collect(events: AsyncIterable): Promise { + const out: ChatGroundingEvent[] = []; + for await (const event of events) out.push(event); + return out; +} + +test("REGRESSION: runChatGrounding reports the usage and cost it used to discard", async () => { + const records = recordingSink(); + await collect( + runChatGrounding(USER_ONLY, { + env: { MINER_CODING_AGENT_PROVIDER: "agent-sdk" }, + query: queryYielding([ + { type: "assistant", message: { content: [{ type: "text", text: "idle" }] } }, + { type: "result", subtype: "success", total_cost_usd: 0.031, usage: { input_tokens: 1800, output_tokens: 240 } }, + ]), + }), + ); + assert.equal(records.length, 1); + assert.equal(records[0]?.provider, CHAT_GROUNDING_PROVIDER); + assert.equal(records[0]?.inputTokens, 1800); + assert.equal(records[0]?.outputTokens, 240); + assert.equal(records[0]?.totalTokens, 2040); + assert.equal(records[0]?.totalCostUsd, 0.031); + assert.equal(records[0]?.isError, false); +}); + +test("runChatGrounding reports a stream with no result frame as a failed generation", async () => { + const records = recordingSink(); + await collect( + runChatGrounding(USER_ONLY, { + env: { MINER_CODING_AGENT_PROVIDER: "agent-sdk" }, + query: queryYielding([{ type: "assistant", message: { content: [{ type: "text", text: "partial" }] } }]), + }), + ); + assert.equal(records[0]?.isError, true); + assert.equal((records[0]?.error as Error).message, "chat_grounding_no_result"); +}); + +test("runChatGrounding emits nothing on the fail-closed provider path -- no model was reached", async () => { + const records = recordingSink(); + await collect(runChatGrounding(USER_ONLY, { env: {} })); + assert.equal(records.length, 0); +}); diff --git a/packages/loopover-miner/lib/coding-agent-construction.ts b/packages/loopover-miner/lib/coding-agent-construction.ts index cf1f10b76..d49c909ac 100644 --- a/packages/loopover-miner/lib/coding-agent-construction.ts +++ b/packages/loopover-miner/lib/coding-agent-construction.ts @@ -9,7 +9,6 @@ import { spawn as nodeSpawn } from "node:child_process"; import { - CODING_AGENT_DRIVER_CONFIG_ENV, createCodingAgentDriver, resolveFirstConfiguredCodingAgentDriverName, type AgentSdkHooks, @@ -22,7 +21,6 @@ import { type HouseRulesConfig, type HouseRulesOptions, } from "./coding-agent-house-rules.js"; -import { captureMinerPostHogAiGeneration } from "./posthog.js"; /** * Real `child_process.spawn`-backed implementation of the engine's `CliSubprocessSpawnFn` contract. Captures @@ -73,48 +71,6 @@ export function createRealCliSubprocessSpawn(): CliSubprocessSpawnFn { }); } -/** Wrap a real `CodingAgentDriver` with PostHog `$ai_generation` capture (#8296 AMS follow-up, epic #8286). - * Lives here, not in `@loopover/engine` (the pure driver package cannot depend on `posthog-node` or any - * vendor client -- the same "no cross-package import" boundary this file's own header already documents - * for its spawn implementation): this is the miner CLI's own host-bound construction site, exactly where - * `src/selfhost/`'s equivalent ORB-side wrapper (`withAiGenerationCapture`, ai.ts) lives relative to its - * own chain. `CodingAgentDriverResult` carries the blended `costUsd`/`tokensUsed` plus the input/output - * split when the provider reported one (#10198); all of it is forwarded verbatim, and a driver that knows - * no split simply leaves those fields absent rather than having one fabricated. A driver failure is reported via - * `result.ok === false` (the real, observed contract every shipped driver follows -- none of them throw - * for an ordinary task failure), with a genuine thrown exception handled defensively on top. */ -export function withCodingAgentAiGenerationCapture(providerName: string, model: string, driver: CodingAgentDriver): CodingAgentDriver { - return { - async run(task) { - const startedAtMs = Date.now(); - try { - const result = await driver.run(task); - captureMinerPostHogAiGeneration({ - provider: providerName, - model, - latencyMs: Date.now() - startedAtMs, - isError: !result.ok, - totalTokens: result.tokensUsed, - inputTokens: result.inputTokens, - outputTokens: result.outputTokens, - totalCostUsd: result.costUsd, - error: result.ok ? undefined : result.error, - }); - return result; - } catch (error) { - captureMinerPostHogAiGeneration({ - provider: providerName, - model, - latencyMs: Date.now() - startedAtMs, - isError: true, - error, - }); - throw error; - } - }, - }; -} - export type ConstructProductionCodingAgentDriverOptions = { spawn?: CliSubprocessSpawnFn; query?: AgentSdkQueryFn; @@ -152,7 +108,12 @@ export function constructProductionCodingAgentDriver( (providerName.trim().toLowerCase() === "agent-sdk" ? buildHouseRulesAgentSdkHooks(options.houseRulesConfig, options.houseRulesOptions) : undefined); - const driver = createCodingAgentDriver({ + // #10200: `$ai_generation` capture is no longer attached here. It moved INTO `createCodingAgentDriver` + // (packages/loopover-engine/src/miner/driver-factory.ts) — the one chokepoint every real driver is built + // through — because attaching it at this construction site left `runCodingAgentAttempt`'s own path, which + // calls that factory directly, silently uncaptured. The host half of the seam is now + // `setMinerAiGenerationSink`, registered once by `initMinerPostHog` (posthog.ts). + return createCodingAgentDriver({ providerName, env, spawn: options.spawn ?? createRealCliSubprocessSpawn(), @@ -160,11 +121,4 @@ export function constructProductionCodingAgentDriver( ...(hooks !== undefined ? { hooks } : {}), ...(options.listChangedFiles !== undefined ? { listChangedFiles: options.listChangedFiles } : {}), }); - // #8296 AMS follow-up: the configured model env var (CODING_AGENT_DRIVER_CONFIG_ENV), when this - // provider declares one -- agent-sdk declares none (its session uses the account/CLI default), so it - // falls back to the provider name itself, mirroring ai.ts's own "unconfigured -> falls back to a - // sensible default" convention. - const modelEnvKey = CODING_AGENT_DRIVER_CONFIG_ENV[providerName as keyof typeof CODING_AGENT_DRIVER_CONFIG_ENV]?.model; - const model = (modelEnvKey ? env[modelEnvKey] : undefined) || providerName; - return withCodingAgentAiGenerationCapture(providerName, model, driver); } diff --git a/packages/loopover-miner/lib/posthog.ts b/packages/loopover-miner/lib/posthog.ts index 21b1aa3fd..0285692a0 100644 --- a/packages/loopover-miner/lib/posthog.ts +++ b/packages/loopover-miner/lib/posthog.ts @@ -14,6 +14,7 @@ * proves for opt-in CLI telemetry. */ import { randomUUID } from "node:crypto"; +import { setMinerAiGenerationSink } from "@loopover/engine"; type PostHogNs = typeof import("posthog-node"); type PostHogClient = InstanceType; @@ -60,6 +61,11 @@ export async function initMinerPostHog(env: Record = const host = env.LOOPOVER_MINER_POSTHOG_HOST ?? DEFAULT_POSTHOG_HOST; client = new PostHog(apiKey, { host, flushAt: 1, flushInterval: 0 }); active = true; + // #10200: the host half of @loopover/engine's generation seam. Registered HERE, on the opt-in path only, so + // the no-phone-home default (#6011) holds by construction -- an operator who never set the API key leaves the + // engine with no sink at all, and `emitMinerAiGeneration` no-ops. Registering it once here is also what makes + // the capture unbypassable: no driver construction site can opt out of a sink it never had to attach. + setMinerAiGenerationSink(captureMinerPostHogAiGeneration); return true; } @@ -147,13 +153,15 @@ export function captureMinerPostHogAiGeneration(event: MinerAiGenerationEvent): // PostHog's own $ai_generation schema reports latency in SECONDS, not ms. $ai_latency: event.latencyMs / 1000, $ai_http_status: event.isError ? 500 : 200, - // #10198: the provider's real split when it reported one. 0 remains the honest fallback for a provider - // that only ever reports a blended total -- it means "no split known", and `tokens_used` below still - // carries the figure that IS known. - $ai_input_tokens: Number.isFinite(event.inputTokens) ? event.inputTokens : 0, - $ai_output_tokens: Number.isFinite(event.outputTokens) ? event.outputTokens : 0, $ai_is_error: event.isError, }; + // #10207: OMITTED, not zeroed, when the provider reported no split. A fabricated 0 is indistinguishable from + // a real 0 once it is in an aggregate -- it drags every tokens-per-call and input:output ratio toward zero and + // silently understates them, which is the same reasoning #10198 applied to the split itself and this file's + // own `tokens_used`/`$ai_total_cost_usd` already followed. A provider that only ever reports a blended total + // now contributes nothing to the token properties rather than a run of false zeros. + if (Number.isFinite(event.inputTokens)) properties.$ai_input_tokens = event.inputTokens; + if (Number.isFinite(event.outputTokens)) properties.$ai_output_tokens = event.outputTokens; if (Number.isFinite(event.totalTokens)) properties.tokens_used = event.totalTokens; if (Number.isFinite(event.totalCostUsd)) properties.$ai_total_cost_usd = event.totalCostUsd; if (event.isError) { @@ -167,8 +175,11 @@ export function captureMinerPostHogAiGeneration(event: MinerAiGenerationEvent): } } -/** Test-only: reset module state so one test's activation can't leak into the next. */ +/** Test-only: reset module state so one test's activation can't leak into the next. Clears the engine-side sink + * too (#10200) -- it is process-wide module state exactly like `client`/`active`, so leaving it registered + * would let one test's activation keep capturing into the next test's client. */ export function resetMinerPostHogForTesting(): void { client = undefined; active = false; + setMinerAiGenerationSink(undefined); } diff --git a/src/selfhost/posthog.ts b/src/selfhost/posthog.ts index a867ab64d..ed54dc352 100644 --- a/src/selfhost/posthog.ts +++ b/src/selfhost/posthog.ts @@ -522,11 +522,18 @@ export function capturePostHogAiGeneration(event: PostHogAiGenerationEvent): voi // PostHog's own $ai_generation schema reports latency in SECONDS, not ms. $ai_latency: event.latencyMs / 1000, $ai_http_status: event.isError ? 500 : 200, - $ai_input_tokens: Number.isFinite(event.inputTokens) ? event.inputTokens : 0, - $ai_output_tokens: Number.isFinite(event.outputTokens) ? event.outputTokens : 0, $ai_is_error: event.isError, environment: posthogEnvironment, }; + // #10207: OMITTED, not zeroed, when the provider reported no usage. A fabricated 0 is indistinguishable from a + // real 0 once it is in an aggregate -- it drags every tokens-per-call and input:output ratio toward zero and + // silently understates them. The providers that DO report usage are the majority here (ai_usage_events records + // a real split for ollama, claude-code and codex), so the zeros were coming from the handful that genuinely + // report none -- Workers AI among them -- and were being averaged in as if they were measurements. + // `$ai_total_cost_usd` directly below has always been conditional for exactly this reason; these two were the + // outliers. A genuinely reported 0 still lands, because absence is tested, not falsiness. + if (Number.isFinite(event.inputTokens)) properties.$ai_input_tokens = event.inputTokens; + if (Number.isFinite(event.outputTokens)) properties.$ai_output_tokens = event.outputTokens; if (Number.isFinite(event.totalCostUsd)) properties.$ai_total_cost_usd = event.totalCostUsd; if (event.effort) properties.$ai_model_parameters = { effort: event.effort }; if (event.isError) { diff --git a/test/unit/chat-grounding-engine.test.ts b/test/unit/chat-grounding-engine.test.ts index 01a488814..0bffa06db 100644 --- a/test/unit/chat-grounding-engine.test.ts +++ b/test/unit/chat-grounding-engine.test.ts @@ -1,8 +1,9 @@ -import { describe, expect, it } from "vitest"; +import { afterEach, describe, expect, it } from "vitest"; import { buildChatPrompt, CHAT_GROUNDING_MCP_SERVER_NAME, + CHAT_GROUNDING_PROVIDER, CHAT_GROUNDING_TOOL_NAMES, CHAT_REDACTED_TEXT, CHAT_SYSTEM_PROMPT, @@ -12,9 +13,11 @@ import { resolveChatProviderError, resolveChatQuery, runChatGrounding, + setMinerAiGenerationSink, type ChatGroundingEvent, type ChatMessage, type ChatQueryFn, + type MinerAiGenerationRecord, } from "../../packages/loopover-engine/src/index"; // Vitest mirror of packages/loopover-engine/test/chat-grounding.test.ts (#6517). codecov/patch is computed from @@ -359,3 +362,97 @@ describe("chat grounding seam resolution (#6517)", () => { ]); }); }); + +describe("chat grounding $ai_generation telemetry (#10200)", () => { + function resultFrame(extra: Record = {}): Record { + return { type: "result", subtype: "success", total_cost_usd: 0.031, usage: { input_tokens: 1800, output_tokens: 240 }, ...extra }; + } + + function recordingSink(): MinerAiGenerationRecord[] { + const records: MinerAiGenerationRecord[] = []; + setMinerAiGenerationSink((record) => records.push(record)); + return records; + } + + afterEach(() => { + setMinerAiGenerationSink(undefined); + }); + + it("REGRESSION: reports the session's real usage and cost from the result frame it used to discard", async () => { + // The driver read only `assistant`/`user` messages, so the SDK's own usage and cost were on the wire and + // thrown away — the session was real spend and produced no telemetry at all. + const records = recordingSink(); + const events = await collect( + runChatGrounding(USER_ONLY, { env: AGENT_SDK_ENV, query: queryYielding([assistantText("your run state is idle"), resultFrame()]) }), + ); + expect(records).toHaveLength(1); + expect(records[0]?.provider).toBe(CHAT_GROUNDING_PROVIDER); + expect(records[0]?.model).toBe("agent-sdk"); + expect(records[0]?.isError).toBe(false); + expect(records[0]?.inputTokens).toBe(1800); + expect(records[0]?.outputTokens).toBe(240); + expect(records[0]?.totalTokens).toBe(2040); + expect(records[0]?.totalCostUsd).toBe(0.031); + expect(records[0]?.error).toBeUndefined(); + expect(records[0]?.latencyMs).toBeGreaterThanOrEqual(0); + // The result frame carries usage, never conversational content, so it must not become a wire event. + expect(events.filter((event) => event.type === "text")).toHaveLength(1); + expect(events[events.length - 1]).toEqual({ type: "done" }); + }); + + it("omits usage the frame did not carry rather than fabricating zeros (#10207)", async () => { + const records = recordingSink(); + await collect( + runChatGrounding(USER_ONLY, { + env: AGENT_SDK_ENV, + query: queryYielding([resultFrame({ usage: undefined, total_cost_usd: undefined })]), + }), + ); + expect(records[0]?.inputTokens).toBeUndefined(); + expect(records[0]?.outputTokens).toBeUndefined(); + expect(records[0]?.totalTokens).toBeUndefined(); + expect(records[0]?.totalCostUsd).toBeUndefined(); + expect(records[0]?.isError).toBe(false); + }); + + it.each([ + ["a stream that ends with no result frame", [assistantText("partial")], "chat_grounding_no_result"], + ["a result frame flagged is_error", [resultFrame({ is_error: true })], "chat_grounding_errored"], + ["a non-success subtype", [resultFrame({ subtype: "error_max_turns" })], "chat_grounding_error_max_turns"], + ["a non-string subtype", [resultFrame({ subtype: 7 })], "chat_grounding_unknown"], + ])("reports %s as a failed generation", async (_label, messages, expectedError) => { + const records = recordingSink(); + await collect(runChatGrounding(USER_ONLY, { env: AGENT_SDK_ENV, query: queryYielding(messages) })); + expect(records).toHaveLength(1); + expect(records[0]?.isError).toBe(true); + expect((records[0]?.error as Error).message).toBe(expectedError); + }); + + it("captures the thrown-SDK path as a failure, alongside the error wire event", async () => { + const records = recordingSink(); + const boom: ChatQueryFn = () => + (async function* () { + throw new Error("sdk session refused"); + })(); + const events = await collect(runChatGrounding(USER_ONLY, { env: AGENT_SDK_ENV, query: boom })); + expect(records).toHaveLength(1); + expect(records[0]?.isError).toBe(true); + expect((records[0]?.error as Error).message).toBe("sdk session refused"); + expect(events).toContainEqual({ type: "error", code: "chat_grounding_failed", message: "sdk session refused" }); + }); + + it("emits NOTHING on the fail-closed provider path — no model was reached", async () => { + // A provider-config refusal is a request that produced no generation; reporting one would fabricate spend + // that never happened. (The ORB gives that case its own separate `selfhost_ai_degraded` event, #10186.) + const records = recordingSink(); + await collect(runChatGrounding(USER_ONLY, { env: {} })); + await collect(runChatGrounding(USER_ONLY, { env: { MINER_CODING_AGENT_PROVIDER: "claude-cli" } })); + expect(records).toHaveLength(0); + }); + + it("stays a silent no-op when no host sink is registered", async () => { + await expect( + collect(runChatGrounding(USER_ONLY, { env: AGENT_SDK_ENV, query: queryYielding([resultFrame()]) })), + ).resolves.toContainEqual({ type: "done" }); + }); +}); diff --git a/test/unit/miner-ai-generation-sink.test.ts b/test/unit/miner-ai-generation-sink.test.ts new file mode 100644 index 000000000..0a36f96c1 --- /dev/null +++ b/test/unit/miner-ai-generation-sink.test.ts @@ -0,0 +1,178 @@ +import { afterEach, describe, expect, it } from "vitest"; + +import { + createCodingAgentDriver, + emitMinerAiGeneration, + hasMinerAiGenerationSink, + resolveCodingAgentTelemetryModel, + runCodingAgentAttempt, + setMinerAiGenerationSink, + withCodingAgentGenerationCapture, + type CodingAgentDriver, + type CodingAgentDriverTask, + type MinerAiGenerationRecord, +} from "../../packages/loopover-engine/src/index"; + +// Vitest mirror of packages/loopover-engine/test/ai-generation-sink.test.ts (#10200). codecov/patch is computed +// from this app vitest run (vitest.config coverage includes packages/loopover-engine/src/**), so the changed +// engine lines need a vitest test that imports the SRC directly — the engine's own node:test suite is not +// collected here. + +const task: CodingAgentDriverTask = { + attemptId: "attempt-sink-1", + workingDirectory: "/tmp/worktrees/attempt-sink-1", + acceptanceCriteriaPath: "/tmp/worktrees/attempt-sink-1/ACCEPTANCE-CRITERIA.md", + instructions: "Apply the fix described in ACCEPTANCE-CRITERIA.md.", + maxTurns: 4, +}; + +/** Registers a recording sink and returns the records array it appends to. */ +function recordingSink(): MinerAiGenerationRecord[] { + const records: MinerAiGenerationRecord[] = []; + setMinerAiGenerationSink((record) => records.push(record)); + return records; +} + +afterEach(() => { + setMinerAiGenerationSink(undefined); +}); + +describe("miner AI generation sink (#10200)", () => { + it("is unregistered by default, so emitting is a silent no-op", () => { + expect(hasMinerAiGenerationSink()).toBe(false); + // The assertion is that this does not throw with no sink registered. + expect(() => emitMinerAiGeneration({ provider: "agent-sdk", model: "agent-sdk", latencyMs: 1, isError: false })).not.toThrow(); + }); + + it("forwards a record to a registered sink, and stops once it is cleared", () => { + const records = recordingSink(); + expect(hasMinerAiGenerationSink()).toBe(true); + emitMinerAiGeneration({ provider: "claude-cli", model: "claude-sonnet-5", latencyMs: 7, isError: false }); + expect(records).toHaveLength(1); + expect(records[0]?.provider).toBe("claude-cli"); + + setMinerAiGenerationSink(undefined); + expect(hasMinerAiGenerationSink()).toBe(false); + emitMinerAiGeneration({ provider: "claude-cli", model: "claude-sonnet-5", latencyMs: 7, isError: false }); + expect(records).toHaveLength(1); + }); + + it("swallows a throwing sink — telemetry must never crash the AI call it instruments", () => { + setMinerAiGenerationSink(() => { + throw new Error("posthog exploded"); + }); + expect(() => emitMinerAiGeneration({ provider: "agent-sdk", model: "agent-sdk", latencyMs: 1, isError: true })).not.toThrow(); + }); +}); + +describe("resolveCodingAgentTelemetryModel (#10200)", () => { + it("reads the provider's own configured model env var when it declares one", () => { + expect(resolveCodingAgentTelemetryModel("claude-cli", { MINER_CODING_AGENT_CLAUDE_MODEL: "claude-opus-5" })).toBe("claude-opus-5"); + expect(resolveCodingAgentTelemetryModel("codex-cli", { MINER_CODING_AGENT_CODEX_MODEL: "gpt-5-codex" })).toBe("gpt-5-codex"); + }); + + it("falls back to the provider name when the provider declares no model key, or the value is blank", () => { + // agent-sdk declares none — its session uses the account/CLI default. + expect(resolveCodingAgentTelemetryModel("agent-sdk", {})).toBe("agent-sdk"); + expect(resolveCodingAgentTelemetryModel("claude-cli", {})).toBe("claude-cli"); + // Whitespace-only is not a model id: firstConfiguredEnvValue trims, so it degrades to the provider name + // rather than reaching PostHog as a blank string. + expect(resolveCodingAgentTelemetryModel("claude-cli", { MINER_CODING_AGENT_CLAUDE_MODEL: " " })).toBe("claude-cli"); + }); +}); + +describe("createCodingAgentDriver attaches capture at the single chokepoint (#10200)", () => { + const spawnOk = async () => ({ stdout: "done", code: 0 }); + + it("REGRESSION: captures an attempt built through runCodingAgentAttempt, which bypassed the host wrapper", async () => { + // The exact bypass #10200 names: runCodingAgentAttempt resolves its driver through createCodingAgentDriver + // directly, never through constructProductionCodingAgentDriver, so before the move every attempt it ran was + // uncaptured. Nothing about this call site opts in — the capture is attached inside the factory. + const records = recordingSink(); + await runCodingAgentAttempt({ + providerName: "claude-cli", + env: { MINER_CODING_AGENT_MODE: "live", MINER_CODING_AGENT_CLAUDE_MODEL: "claude-opus-5" }, + task, + spawn: spawnOk, + }); + expect(records).toHaveLength(1); + expect(records[0]?.provider).toBe("claude-cli"); + expect(records[0]?.model).toBe("claude-opus-5"); + expect(records[0]?.isError).toBe(false); + expect(records[0]?.latencyMs).toBeGreaterThanOrEqual(0); + }); + + it("captures a driver built straight from the factory, normalizing the provider name", async () => { + const records = recordingSink(); + const driver = createCodingAgentDriver({ providerName: " CODEX-CLI ", env: {}, spawn: spawnOk }); + await driver.run(task); + expect(records).toHaveLength(1); + expect(records[0]?.provider).toBe("codex-cli"); + expect(records[0]?.model).toBe("codex-cli"); + }); + + it("does NOT capture the noop provider — a stub that never reaches a model has no generation to report", async () => { + const records = recordingSink(); + const driver = createCodingAgentDriver({ providerName: "noop", env: {} }); + await driver.run(task); + expect(records).toHaveLength(0); + }); + + it("does NOT capture an injected test driver — the seam returns it verbatim", async () => { + const records = recordingSink(); + const injected: CodingAgentDriver = { run: async () => ({ ok: true, changedFiles: [], summary: "injected", transcript: "" }) }; + expect(createCodingAgentDriver({ providerName: "claude-cli", driver: injected })).toBe(injected); + await injected.run(task); + expect(records).toHaveLength(0); + }); + + it("does NOT capture a dry-run attempt — it never calls driver.run()", async () => { + const records = recordingSink(); + const { mode } = await runCodingAgentAttempt({ + providerName: "claude-cli", + env: { MINER_CODING_AGENT_MODE: "live" }, + agentDryRun: true, + task, + }); + expect(mode).not.toBe("live"); + expect(records).toHaveLength(0); + }); +}); + +describe("withCodingAgentGenerationCapture record shape (#10200/#10207)", () => { + it("omits every token/cost field the driver did not report, rather than zeroing them", async () => { + const records = recordingSink(); + const driver = withCodingAgentGenerationCapture("codex-cli", "gpt-5-codex", { + run: async () => ({ ok: true, changedFiles: [], summary: "done", transcript: "" }), + }); + await driver.run(task); + expect(records[0]?.totalTokens).toBeUndefined(); + expect(records[0]?.inputTokens).toBeUndefined(); + expect(records[0]?.outputTokens).toBeUndefined(); + expect(records[0]?.totalCostUsd).toBeUndefined(); + expect(records[0]?.error).toBeUndefined(); + }); + + it("carries the driver's own error string on an ok:false result, without throwing", async () => { + const records = recordingSink(); + const driver = withCodingAgentGenerationCapture("codex-cli", "gpt-5-codex", { + run: async () => ({ ok: false, changedFiles: [], summary: "failed", transcript: "", error: "codex_timeout_120000ms" }), + }); + const result = await driver.run(task); + expect(result.ok).toBe(false); + expect(records[0]?.isError).toBe(true); + expect(records[0]?.error).toBe("codex_timeout_120000ms"); + }); + + it("captures and rethrows when the wrapped driver throws", async () => { + const records = recordingSink(); + const driver = withCodingAgentGenerationCapture("agent-sdk", "agent-sdk", { + run: async () => { + throw new Error("sdk crashed"); + }, + }); + await expect(driver.run(task)).rejects.toThrow("sdk crashed"); + expect(records[0]?.isError).toBe(true); + expect((records[0]?.error as Error).message).toBe("sdk crashed"); + }); +}); diff --git a/test/unit/miner-coding-agent-construction.test.ts b/test/unit/miner-coding-agent-construction.test.ts index 4c48b12d1..285a75037 100644 --- a/test/unit/miner-coding-agent-construction.test.ts +++ b/test/unit/miner-coding-agent-construction.test.ts @@ -17,9 +17,9 @@ vi.mock("posthog-node", () => ({ PostHog: posthogMock.PostHog })); import { createRealCliSubprocessSpawn, constructProductionCodingAgentDriver, - withCodingAgentAiGenerationCapture, } from "../../packages/loopover-miner/lib/coding-agent-construction"; import { initMinerPostHog, resetMinerPostHogForTesting } from "../../packages/loopover-miner/lib/posthog"; +import { withCodingAgentGenerationCapture } from "../../packages/loopover-engine/src/index"; import type { AgentSdkQueryFn, CodingAgentDriver, CodingAgentDriverTask } from "../../packages/loopover-engine/src/index"; beforeEach(() => { @@ -199,13 +199,16 @@ describe("constructProductionCodingAgentDriver (#5131)", () => { }); }); -describe("withCodingAgentAiGenerationCapture (#8296 AMS follow-up)", () => { +// #10200: the wrapper itself moved into @loopover/engine (ai-generation-sink.ts) so the engine's own driver +// factory can apply it; these cases still drive it end-to-end through the miner's real PostHog sink, which is +// what they were always actually asserting. +describe("withCodingAgentGenerationCapture (#8296 AMS follow-up, relocated by #10200)", () => { function driverReturning(result: Awaited>): CodingAgentDriver { return { run: async () => result }; } it("stays a no-op end-to-end when PostHog is unconfigured", async () => { - const driver = withCodingAgentAiGenerationCapture("claude-cli", "claude-sonnet-5", driverReturning({ ok: true, changedFiles: [], summary: "done", transcript: "" })); + const driver = withCodingAgentGenerationCapture("claude-cli", "claude-sonnet-5", driverReturning({ ok: true, changedFiles: [], summary: "done", transcript: "" })); const result = await driver.run(task); expect(result.ok).toBe(true); expect(posthogMock.capture).not.toHaveBeenCalled(); @@ -213,7 +216,7 @@ describe("withCodingAgentAiGenerationCapture (#8296 AMS follow-up)", () => { it("forwards the driver's cost, blended tokens AND input/output split to the capture (#10198)", async () => { await initMinerPostHog({ LOOPOVER_MINER_POSTHOG_API_KEY: "phc_test_key" }); - const driver = withCodingAgentAiGenerationCapture( + const driver = withCodingAgentGenerationCapture( "claude-cli", "claude-sonnet-5", driverReturning({ ok: true, changedFiles: ["a.ts"], summary: "done", transcript: "", costUsd: 0.12, tokensUsed: 4000, inputTokens: 3200, outputTokens: 800 }), @@ -230,9 +233,9 @@ describe("withCodingAgentAiGenerationCapture (#8296 AMS follow-up)", () => { expect(properties.$ai_total_cost_usd).toBe(0.12); }); - it("leaves the split at 0 for a driver that reports only a blended total (#10198)", async () => { + it("REGRESSION (#10207): OMITS the split for a driver that reports only a blended total -- never a fabricated 0", async () => { await initMinerPostHog({ LOOPOVER_MINER_POSTHOG_API_KEY: "phc_test_key" }); - const driver = withCodingAgentAiGenerationCapture( + const driver = withCodingAgentGenerationCapture( "codex-cli", "gpt-5-codex", driverReturning({ ok: true, changedFiles: [], summary: "done", transcript: "", tokensUsed: 4000 }), @@ -240,13 +243,27 @@ describe("withCodingAgentAiGenerationCapture (#8296 AMS follow-up)", () => { await driver.run(task); const { properties } = posthogMock.capture.mock.calls[0]?.[0]; expect(properties.tokens_used).toBe(4000); + expect(properties).not.toHaveProperty("$ai_input_tokens"); + expect(properties).not.toHaveProperty("$ai_output_tokens"); + }); + + it("still reports a GENUINELY reported 0 -- absence is what is tested, not falsiness (#10207)", async () => { + await initMinerPostHog({ LOOPOVER_MINER_POSTHOG_API_KEY: "phc_test_key" }); + const driver = withCodingAgentGenerationCapture( + "codex-cli", + "gpt-5-codex", + driverReturning({ ok: true, changedFiles: [], summary: "done", transcript: "", tokensUsed: 0, inputTokens: 0, outputTokens: 0 }), + ); + await driver.run(task); + const { properties } = posthogMock.capture.mock.calls[0]?.[0]; expect(properties.$ai_input_tokens).toBe(0); expect(properties.$ai_output_tokens).toBe(0); + expect(properties.tokens_used).toBe(0); }); it("captures result.ok:false as a failure, using the driver's own error string -- no exception thrown", async () => { await initMinerPostHog({ LOOPOVER_MINER_POSTHOG_API_KEY: "phc_test_key" }); - const driver = withCodingAgentAiGenerationCapture( + const driver = withCodingAgentGenerationCapture( "codex-cli", "gpt-5-codex", driverReturning({ ok: false, changedFiles: [], summary: "failed", transcript: "", error: "codex_timeout_120000ms" }), @@ -261,7 +278,7 @@ describe("withCodingAgentAiGenerationCapture (#8296 AMS follow-up)", () => { it("REGRESSION: still captures a failure AND rethrows when the wrapped driver itself throws unexpectedly", async () => { await initMinerPostHog({ LOOPOVER_MINER_POSTHOG_API_KEY: "phc_test_key" }); - const driver = withCodingAgentAiGenerationCapture("agent-sdk", "agent-sdk", { run: async () => { throw new Error("sdk crashed"); } }); + const driver = withCodingAgentGenerationCapture("agent-sdk", "agent-sdk", { run: async () => { throw new Error("sdk crashed"); } }); await expect(driver.run(task)).rejects.toThrow("sdk crashed"); const { properties } = posthogMock.capture.mock.calls[0]?.[0]; expect(properties.$ai_is_error).toBe(true); diff --git a/test/unit/miner-posthog.test.ts b/test/unit/miner-posthog.test.ts index 43b22a71c..4c90a091c 100644 --- a/test/unit/miner-posthog.test.ts +++ b/test/unit/miner-posthog.test.ts @@ -177,19 +177,31 @@ describe("loopover-miner opt-in PostHog (#8292, epic #8286)", () => { expect("$ai_output_choices" in call.properties).toBe(false); }); - it("falls back to 0 for a side the driver could not report, without losing the blended total (#10198)", async () => { - // A provider that reports only a blended total genuinely has no split; 0 here means "no split known", - // and tokens_used still carries the figure that IS known. + it("REGRESSION (#10207): omits a side the driver could not report, without losing the blended total", async () => { + // A provider that reports only a blended total genuinely has no split. #10198 sent 0 for it, meaning "no + // split known" -- but PostHog cannot tell that apart from a measured 0, so those zeros were averaged into + // every tokens-per-call and input:output figure as if they were data. Absent now; tokens_used still + // carries the figure that IS known. await initMinerPostHog({ LOOPOVER_MINER_POSTHOG_API_KEY: "phc_test_key" }); captureMinerPostHogAiGeneration({ ...BASE, totalTokens: 999 }); captureMinerPostHogAiGeneration({ ...BASE, totalTokens: 999, inputTokens: 800, outputTokens: Number.NaN }); const blended = posthogMock.capture.mock.calls[0]?.[0].properties; - expect(blended.$ai_input_tokens).toBe(0); - expect(blended.$ai_output_tokens).toBe(0); + expect("$ai_input_tokens" in blended).toBe(false); + expect("$ai_output_tokens" in blended).toBe(false); expect(blended.tokens_used).toBe(999); + // A partially-reported split keeps the side that IS real and drops only the unreported one. const partial = posthogMock.capture.mock.calls[1]?.[0].properties; expect(partial.$ai_input_tokens).toBe(800); - expect(partial.$ai_output_tokens).toBe(0); + expect("$ai_output_tokens" in partial).toBe(false); + }); + + it("still reports a GENUINELY measured 0 -- the omission tests absence, not falsiness (#10207)", async () => { + await initMinerPostHog({ LOOPOVER_MINER_POSTHOG_API_KEY: "phc_test_key" }); + captureMinerPostHogAiGeneration({ ...BASE, totalTokens: 0, inputTokens: 0, outputTokens: 0 }); + const { properties } = posthogMock.capture.mock.calls[0]?.[0]; + expect(properties.$ai_input_tokens).toBe(0); + expect(properties.$ai_output_tokens).toBe(0); + expect(properties.tokens_used).toBe(0); }); it("omits tokens_used/$ai_total_cost_usd when neither is supplied or finite", async () => { diff --git a/test/unit/selfhost-ai.test.ts b/test/unit/selfhost-ai.test.ts index 734dd0641..d80647a76 100644 --- a/test/unit/selfhost-ai.test.ts +++ b/test/unit/selfhost-ai.test.ts @@ -720,7 +720,10 @@ describe("$ai_generation PostHog capture at the chain chokepoint (#8296)", () => await createChainAi([provider]).run("m", { prompt: "x" }); const { properties } = posthogMocks.capture.mock.calls[0]?.[0]; expect(properties.$ai_provider).toBe("posthog-no-usage-provider"); - expect(properties.$ai_input_tokens).toBe(0); + // #10207: a provider that reported no usage contributes NO token properties, rather than a fabricated 0 + // that PostHog cannot tell apart from a measured one. The provider name still resolves, which is what this + // case is actually about. + expect("$ai_input_tokens" in properties).toBe(false); }); it("captures a failed generation with $ai_is_error/$ai_error on a real chain failure", async () => { diff --git a/test/unit/selfhost-posthog.test.ts b/test/unit/selfhost-posthog.test.ts index d96ce5de9..0e92e9933 100644 --- a/test/unit/selfhost-posthog.test.ts +++ b/test/unit/selfhost-posthog.test.ts @@ -680,12 +680,28 @@ describe("capturePostHogAiGeneration (#8296)", () => { expect(mocks.capture.mock.calls[0]?.[0].event).toBe("$ai_embedding"); }); - it("defaults input/output tokens to 0 when omitted or non-finite", async () => { + it("REGRESSION (#10207): omits input/output tokens when omitted or non-finite -- never a fabricated 0", async () => { + // A provider that reports no usage (Workers AI among them) used to contribute a 0 to both properties, which + // PostHog cannot tell apart from a measured 0 -- so every tokens-per-call and input:output ratio was pulled + // toward zero by calls that had never measured anything. Absence must stay absent. await initPostHog({ POSTHOG_API_KEY: "phc_test_key" } as unknown as NodeJS.ProcessEnv); capturePostHogAiGeneration({ ...BASE, inputTokens: undefined, outputTokens: Number.NaN }); const { properties } = mocks.capture.mock.calls[0]?.[0]; - expect(properties.$ai_input_tokens).toBe(0); - expect(properties.$ai_output_tokens).toBe(0); + expect("$ai_input_tokens" in properties).toBe(false); + expect("$ai_output_tokens" in properties).toBe(false); + }); + + it("keeps a partially-reported split's real side, and a genuinely measured 0 (#10207)", async () => { + await initPostHog({ POSTHOG_API_KEY: "phc_test_key" } as unknown as NodeJS.ProcessEnv); + capturePostHogAiGeneration({ ...BASE, inputTokens: 512, outputTokens: undefined }); + capturePostHogAiGeneration({ ...BASE, inputTokens: 0, outputTokens: 0 }); + const partial = mocks.capture.mock.calls[0]?.[0].properties; + expect(partial.$ai_input_tokens).toBe(512); + expect("$ai_output_tokens" in partial).toBe(false); + // The omission tests absence, not falsiness: a provider that really measured 0 still reports 0. + const measuredZero = mocks.capture.mock.calls[1]?.[0].properties; + expect(measuredZero.$ai_input_tokens).toBe(0); + expect(measuredZero.$ai_output_tokens).toBe(0); }); it("omits $ai_total_cost_usd when cost is not supplied or non-finite", async () => {