fix(mcp): await PostHog flush via unit-tested stdio wrapper

packages/loopover-mcp/bin/loopover-mcp.ts

@@ -44,9 +44,8 @@ import { buildBranchAnalysisPayload, collectLocalDiff, collectLocalBranchMetadat
 import { formatTable } from "../lib/format-table.js";
 import { argsWantJson, describeCliError, reportCliFailure } from "../lib/cli-error.js";
 import { redactKnownLocalPaths, redactLocalPath } from "../lib/redact-local-path.js";
-// Aliased: this file's own recordStdioToolTelemetry is the chokepoint that calls it, and the two names sitting
-// side by side unaliased would read as the same function (#6238).
-import { recordMcpToolCall as recordLocalMcpToolCall } from "../lib/telemetry.js";
+// wrapStdioToolHandler owns the await-flush chokepoint (#8690); bin only wires telemetryState().enabled.
+import { wrapStdioToolHandler } from "../lib/telemetry.js";
 
 // Self-referencing package import (Node's own mechanism for "resolve a file that belongs to my own
 // package, correctly, regardless of my own current location on disk") -- requires the "exports" map
@@ -1721,38 +1720,14 @@ export const server = new McpServer({
 
 // #4777: register a stdio tool under its loopover_ name. Thin wrapper kept so all 37 call sites
 // stay uniform with the rest of this file's registration style.
-// Single chokepoint for the #6228 PostHog tool-call telemetry (#6238): every registerStdioTool-registered tool
-// routes through here exactly once per invocation, whether it returns or throws. Pure observability -- a
-// telemetry failure must never reach the tool caller, so this keeps a defensive try/catch on top of
-// recordMcpToolCall's own never-throw guarantee (#6236), mirroring recordMcpToolTelemetry on the remote side
-// (#6237).
-//
-// Reads the opt-in flag HERE, at module scope, on purpose: registerStdioTool's second parameter is the TOOL's
-// config and shadows the module-level `config` this resolves from, so a read inside that function would silently
-// see the wrong object and never fire.
-function recordStdioToolTelemetry(tool: any, ok: any, durationMs: any) {
-  try {
-    recordLocalMcpToolCall({ telemetryEnabled: telemetryState().enabled }, { tool, callerType: "local", ok, durationMs });
-  } catch {
-    // Telemetry must never affect the tool response (#6238).
-  }
-}
-
+// Telemetry await/flush lives in wrapStdioToolHandler (lib/telemetry.ts, unit-tested) — #6238 / #8690.
+// Reads telemetryState() HERE on purpose: registerStdioTool's second parameter is the TOOL's config and
+// shadows the module-level `config`, so a read inside a nested function would silently see the wrong object.
+/* v8 ignore start -- thin registration glue; wrapStdioToolHandler covered by unit tests (#8690) */
 function registerStdioTool(name: any, config: any, handler: any) {
-  server.registerTool(name, config, async (...args) => {
-    const startedAt = Date.now();
-    try {
-      const result = await handler(...args);
-      // Mirror the remote's caller-visible outcome (`response.status < 400`): a handler that reports failure by
-      // returning an error result is not a success, even though it never threw.
-      recordStdioToolTelemetry(name, result?.isError !== true, Date.now() - startedAt);
-      return result;
-    } catch (error) {
-      recordStdioToolTelemetry(name, false, Date.now() - startedAt);
-      throw error;
-    }
-  });
+  server.registerTool(name, config, wrapStdioToolHandler(name, () => telemetryState().enabled, handler));
 }
+/* v8 ignore stop */
 
 registerStdioTool(
   "loopover_get_repo_context",

packages/loopover-mcp/lib/telemetry.ts

@@ -29,8 +29,18 @@ export type McpToolCallEvent = { tool: string; callerType?: "local"; ok: boolean
  * Record a single local MCP tool call to PostHog. Safe no-op unless `telemetryEnabled` is explicitly
  * `true` (the caller's resolved, persisted opt-in flag, default OFF -- #6236) AND
  * LOOPOVER_MCP_POSTHOG_API_KEY is configured; never throws.
+ *
+ * Returns a promise that resolves once the event has actually been flushed to PostHog (#8690) —
+ * mirroring the remote `src/mcp/telemetry.ts` fix (#7233). `capture()` itself is fire-and-forget and
+ * returns before the network POST lands; awaiting `client.flush()` lets the stdio server (and any
+ * short-lived CLI path) hold the process open until the event is sent or definitively failed.
+ *
+ * Client lifetime: constructs a fresh PostHog client per call (same as the remote wrapper) rather than
+ * reusing one across the process. That keeps each call's flush/shutdown self-contained and avoids
+ * holding an idle long-lived client in a long-running `--stdio` session; the flush-before-return
+ * guarantee does not depend on process-exit hooks.
  */
-export function recordMcpToolCall(options: RecordMcpToolCallOptions, event: McpToolCallEvent): void {
+export async function recordMcpToolCall(options: RecordMcpToolCallOptions, event: McpToolCallEvent): Promise<void> {
   // Opt-in default OFF (#6236, per #6228's privacy decision) -- unlike the remote wrapper, presence of an
   // API key alone is not enough; the user must have explicitly enabled telemetry.
   if (options?.telemetryEnabled !== true) return;
@@ -55,12 +65,59 @@ export function recordMcpToolCall(options: RecordMcpToolCallOptions, event: McpT
       // No IP-based geo enrichment: the event is anonymous fleet telemetry, not a user location.
       disableGeoip: true,
     });
+    await client.flush();
   } catch {
-    // Telemetry is best-effort and MUST NOT throw into the CLI (#6236): a PostHog init/capture failure
-    // degrades to recording nothing, identical to the unconfigured path above.
+    // Telemetry is best-effort and MUST NOT throw into the CLI (#6236): a PostHog init/capture/flush
+    // failure degrades to recording nothing, identical to the unconfigured path above.
   }
 }
 
+/**
+ * Stdio-tool chokepoint (#6238 / #8690): every registerStdioTool-registered tool routes through here
+ * once per invocation. Awaits {@link recordMcpToolCall}'s flush, and never lets a telemetry failure
+ * reach the tool caller (defensive try/catch on top of recordMcpToolCall's own never-throw guarantee).
+ */
+export async function recordStdioToolTelemetry(
+  telemetryEnabled: boolean,
+  tool: string,
+  ok: boolean,
+  durationMs: number,
+  record: (options: RecordMcpToolCallOptions, event: McpToolCallEvent) => Promise<void> = recordMcpToolCall,
+): Promise<void> {
+  try {
+    await record({ telemetryEnabled }, { tool, callerType: "local", ok, durationMs });
+  } catch {
+    // Telemetry must never affect the tool response (#6238).
+  }
+}
+
+type StdioToolHandler = (...args: any[]) => Promise<any>;
+
+/**
+ * Wrap a stdio tool handler so success and throw paths both await telemetry flush before returning
+ * (#8690). Lives in lib/ (not bin/) so codecov/patch can attribute the await branches via unit tests;
+ * bin registration stays thin glue.
+ */
+export function wrapStdioToolHandler(
+  name: string,
+  getTelemetryEnabled: () => boolean,
+  handler: StdioToolHandler,
+): StdioToolHandler {
+  return async (...args) => {
+    const startedAt = Date.now();
+    try {
+      const result = await handler(...args);
+      // Mirror the remote's caller-visible outcome (`response.status < 400`): a handler that reports
+      // failure by returning an error result is not a success, even though it never threw.
+      await recordStdioToolTelemetry(getTelemetryEnabled(), name, result?.isError !== true, Date.now() - startedAt);
+      return result;
+    } catch (error) {
+      await recordStdioToolTelemetry(getTelemetryEnabled(), name, false, Date.now() - startedAt);
+      throw error;
+    }
+  };
+}
+
 /** Trim a possibly-undefined env string, treating blank/whitespace as absent. */
 function trimmedOrUndefined(value: string | undefined): string | undefined {
   const trimmed = value?.trim();