fix(cli): stop background update and skills checks from holding a finished command open (#4556)

* fix(cli): skip background checks in unattended runs and never hold exit on a silent DNS

* fix(cli): probe DNS only on background checks so foreground upgrade and skills commands still wait

* fix(cli): keep update checks for agents and skip them only for CI, dev and the opt-out

* refactor(cli): give the update-check opt-outs one owner shared with auto-install

* docs(cli): stop the auto-install notes claiming runs without a terminal are skipped

* fix(cli): run telemetry flushes one at a time so an overlapping flush never resends an event

* fix(cli): let a telemetry flush run even when the one before it threw

* fix(cli): cap exit wait at one telemetry attempt when an overlapping flush already failed

packages/cli/src/commands/events.test.ts

@@ -0,0 +1,17 @@
+import { describe, expect, it, vi } from "vitest";
+
+const telemetry = vi.hoisted(() => ({
+  trackEvent: vi.fn(),
+  flush: vi.fn(async () => {}),
+  flushSync: vi.fn(),
+}));
+vi.mock("../telemetry/client.js", () => telemetry);
+
+describe("events", () => {
+  it("hands anything flush() left queued to the detached sender", async () => {
+    const { default: events } = await import("./events.js");
+    await events.run?.({ args: { skill: "hyperframes", event: "skill_invoked" } } as never);
+    expect(telemetry.trackEvent).toHaveBeenCalled();
+    expect(telemetry.flushSync).toHaveBeenCalled();
+  });
+});

packages/cli/src/commands/events.ts

@@ -1,5 +1,5 @@
 import { defineCommand } from "citty";
-import { trackEvent, flush } from "../telemetry/client.js";
+import { trackEvent, flush, flushSync } from "../telemetry/client.js";
 import { SKILL_SLUG } from "../telemetry/skill.js";
 
 // Skill-usage telemetry endpoint. A skill reports its own invocation/outcome —
@@ -54,6 +54,7 @@ export default defineCommand({
       }
       trackEvent(event, props);
       await flush();
+      flushSync();
     } catch {
       // swallow — telemetry must never surface a non-zero exit to the caller
     }

packages/cli/src/telemetry/client.test.ts

@@ -7,8 +7,15 @@ vi.stubEnv("HYPERFRAMES_NO_TELEMETRY", "");
 vi.stubEnv("DO_NOT_TRACK", "");
 
 // Pin config so the queue never touches disk and telemetry is enabled.
+const configRead = vi.hoisted(() => ({ failOnce: false }));
 vi.mock("./config.js", () => ({
-  readConfig: () => ({ anonymousId: "anon-test-123", telemetryEnabled: true }),
+  readConfig: () => {
+    if (configRead.failOnce) {
+      configRead.failOnce = false;
+      throw new Error("EACCES");
+    }
+    return { anonymousId: "anon-test-123", telemetryEnabled: true };
+  },
   writeConfig: () => {},
   getIdentityPersistence: () => "durable",
   getIdentityWriteOutcome: () => undefined,
@@ -19,6 +26,9 @@ vi.mock("../utils/env.js", () => ({
   isDevMode: () => false,
 }));
 
+const dns = vi.hoisted(() => ({ answers: true }));
+vi.mock("../utils/hostAnswers.js", () => ({ hostAnswers: async () => dns.answers }));
+
 // Canary enrolment is registry-driven and will change as rollouts ramp; stub
 // it so this asserts the WIRING (does every event carry the cohort?) rather
 // than whichever canaries happen to be live today.
@@ -90,6 +100,21 @@ describe("telemetry queue delivery", () => {
     }
   });
 
+  it("keeps events queued for the exit-time send when DNS does not answer", async () => {
+    dns.answers = false;
+    const fetchMock = vi.fn(() => Promise.resolve(new Response("")));
+    vi.stubGlobal("fetch", fetchMock);
+    try {
+      trackEvent("render_complete", { quality: "draft" });
+      await flush();
+      expect(fetchMock).not.toHaveBeenCalled();
+    } finally {
+      dns.answers = true;
+    }
+    await flush();
+    expect(fetchMock).toHaveBeenCalledTimes(1);
+  });
+
   it("forgets events only after the request completes, and stamps each with a uuid", async () => {
     const fetchMock = vi.fn(() => Promise.resolve(new Response("")));
     vi.stubGlobal("fetch", fetchMock);
@@ -132,6 +157,40 @@ describe("telemetry queue delivery", () => {
     expect(succeeding).toHaveBeenCalledTimes(1);
   });
 
+  it("sends each event once when two flushes overlap", async () => {
+    const pending: Array<(r: Response) => void> = [];
+    const gated = vi.fn(() => new Promise<Response>((res) => pending.push(res)));
+    vi.stubGlobal("fetch", gated);
+
+    trackEvent("render_complete", { quality: "draft" });
+    const eager = flush();
+    const final = flush();
+    await vi.waitFor(() => expect(gated).toHaveBeenCalled());
+    for (const res of pending.splice(0)) res(new Response(""));
+    await Promise.all([eager, final]);
+    expect(gated).toHaveBeenCalledTimes(1);
+  });
+
+  it("leaves the batch to the exit-time send when the flush ahead of it failed", async () => {
+    const fetchMock = vi.fn(() => Promise.reject(new Error("offline")));
+    vi.stubGlobal("fetch", fetchMock);
+    trackEvent("render_complete", { quality: "draft" });
+    await Promise.all([flush(), flush()]);
+    expect(fetchMock).toHaveBeenCalledTimes(1);
+  });
+
+  it("still sends from a flush queued behind one that threw", async () => {
+    const fetchMock = vi.fn(() => Promise.resolve(new Response("")));
+    vi.stubGlobal("fetch", fetchMock);
+    trackEvent("render_complete", { quality: "draft" });
+    configRead.failOnce = true;
+    const first = flush();
+    const second = flush();
+    await expect(first).rejects.toThrow("EACCES");
+    await second;
+    expect(fetchMock).toHaveBeenCalledTimes(1);
+  });
+
   it("does not drop events queued while a flush is in flight", async () => {
     let resolveFetch: (r: Response) => void = () => {};
     const gated = vi.fn(() => new Promise<Response>((res) => (resolveFetch = res)));
@@ -140,6 +199,7 @@ describe("telemetry queue delivery", () => {
     trackEvent("render_complete", { quality: "draft" });
     const inFlight = flush();
     trackEvent("cli_command_result", { command: "render" });
+    await vi.waitFor(() => expect(gated).toHaveBeenCalled());
     resolveFetch(new Response(""));
     await inFlight;
 

packages/cli/src/telemetry/transport.ts

@@ -2,6 +2,7 @@ import { spawn } from "node:child_process";
 import { randomUUID } from "node:crypto";
 import { POSTHOG_API_KEY } from "./posthogKey.js";
 import { readConfig } from "./config.js";
+import { hostAnswers } from "../utils/hostAnswers.js";
 
 // This is a public project API key — safe to embed in client-side code.
 // It only allows writing events, not reading data.
@@ -94,6 +95,8 @@ function buildPayload(events: readonly QueuedEvent[]): string | null {
   return JSON.stringify({ api_key: POSTHOG_API_KEY, batch });
 }
 
+let inFlight: Promise<boolean> | undefined;
+
 /**
  * Flush all queued events to PostHog via async HTTP POST.
  * Call sites: the `beforeExit` hook in cli.ts (normal exit), eager sends right
@@ -110,11 +113,26 @@ function buildPayload(events: readonly QueuedEvent[]): string | null {
  * re-send anything unconfirmed; event uuids make that re-send idempotent.
  */
 export async function flush(): Promise<void> {
+  // One request at a time; one queued behind a failed send leaves the batch to flushSync().
+  const run = inFlight
+    ? inFlight.then((sent) => (sent ? sendQueued() : false), sendQueued)
+    : sendQueued();
+  const current = run.finally(() => {
+    if (inFlight === current) inFlight = undefined;
+  });
+  inFlight = current;
+  await current;
+}
+
+/** Whether nothing is left undelivered from what was queued when it started. */
+async function sendQueued(): Promise<boolean> {
   // Copy, not alias — events queued while the request is in flight must not
   // be swept into the "delivered" set below.
   const snapshot = eventQueue.slice();
   const payload = buildPayload(snapshot);
-  if (payload == null) return;
+  if (payload == null) return true;
+  // Left queued: the exit-time flushSync() sends it from a detached child.
+  if (!(await hostAnswers(new URL(POSTHOG_HOST).hostname))) return false;
 
   const controller = new AbortController();
   const timeout = setTimeout(() => controller.abort(), FLUSH_TIMEOUT_MS);
@@ -130,9 +148,11 @@ export async function flush(): Promise<void> {
     // request was in flight stay for the next flush).
     const sent = new Set(snapshot);
     eventQueue = eventQueue.filter((e) => !sent.has(e));
+    return true;
   } catch {
     // Silently ignore — telemetry must never break the CLI. The events stay
     // queued so the exit-time flushSync() fallback can still deliver them.
+    return false;
   } finally {
     clearTimeout(timeout);
   }

packages/cli/src/utils/autoUpdate.ts

@@ -14,7 +14,7 @@
  * Guardrails:
  *   - Never auto-update across major versions. The user opts in explicitly
  *     via `hyperframes upgrade`.
- *   - Skip on CI, non-TTY, dev mode, unknown installer, ephemeral exec (npx),
+ *   - Skip on CI, dev mode, unknown installer, ephemeral exec (npx),
  *     or when `HYPERFRAMES_NO_AUTO_INSTALL` / `HYPERFRAMES_NO_UPDATE_CHECK`
  *     is set.
  *   - If a previous install is still in flight (less than 10 min old), don't
@@ -30,7 +30,7 @@ import { join } from "node:path";
 import { compareVersions } from "compare-versions";
 import { withFileLock } from "../media-use/lib/config-lock.mjs";
 import { readConfig, writeConfig } from "../telemetry/config.js";
-import { isDevMode } from "./env.js";
+import { updateCheckDisabled } from "./updateCheck.js";
 import {
   detectInstaller,
   installInvocation,
@@ -43,11 +43,7 @@ const LOG_FILE = join(CONFIG_DIR, "auto-update.log");
 const PENDING_TIMEOUT_MS = 10 * 60 * 1000;
 
 function isAutoInstallDisabled(): boolean {
-  if (isDevMode()) return true;
-  if (process.env["CI"] === "true" || process.env["CI"] === "1") return true;
-  if (process.env["HYPERFRAMES_NO_UPDATE_CHECK"] === "1") return true;
-  if (process.env["HYPERFRAMES_NO_AUTO_INSTALL"] === "1") return true;
-  return false;
+  return updateCheckDisabled() || process.env["HYPERFRAMES_NO_AUTO_INSTALL"] === "1";
 }
 
 /** Parse a semver-ish string's major number; returns NaN for pre-releases etc. */

packages/cli/src/utils/hostAnswers.ts

@@ -0,0 +1,21 @@
+import { promises as dns } from "node:dns";
+
+const PROBE_TIMEOUT_MS = 1000;
+
+/**
+ * Whether DNS answers for `host` via c-ares, one short try per nameserver. The system lookup
+ * `fetch` makes cannot be aborted and holds even `process.exit`, so background requests ask this first.
+ */
+export async function hostAnswers(host: string): Promise<boolean> {
+  try {
+    const resolver = new dns.Resolver({ timeout: PROBE_TIMEOUT_MS, tries: 1 });
+    try {
+      await Promise.any([resolver.resolve4(host), resolver.resolve6(host)]);
+      return true;
+    } finally {
+      resolver.cancel();
+    }
+  } catch {
+    return false;
+  }
+}

packages/cli/src/utils/skillsUpdateCheck.test.ts

@@ -17,9 +17,11 @@ vi.mock("../telemetry/config.js", () => ({
   },
 }));
 
+const gate = vi.hoisted(() => ({ suppressed: false, answers: true }));
 vi.mock("./updateCheck.js", () => ({
-  updateNoticesSuppressed: () => false,
+  updateNoticesSuppressed: () => gate.suppressed,
 }));
+vi.mock("./hostAnswers.js", () => ({ hostAnswers: async () => gate.answers }));
 
 const mockCheckSkills = vi.fn();
 vi.mock("./skillsManifest.js", () => ({
@@ -31,9 +33,24 @@ describe("skillsUpdateCheck", () => {
     vi.resetModules();
     config = {};
     mockCheckSkills.mockReset();
+    gate.suppressed = false;
+    gate.answers = true;
+    vi.stubEnv("HYPERFRAMES_SKIP_SKILLS", "");
   });
   afterEach(() => {
     vi.restoreAllMocks();
+    vi.unstubAllEnvs();
+  });
+
+  it.each([
+    ["HYPERFRAMES_SKIP_SKILLS is set", () => vi.stubEnv("HYPERFRAMES_SKIP_SKILLS", "1")],
+    ["the run shows no notices", () => (gate.suppressed = true)],
+    ["DNS does not answer", () => (gate.answers = false)],
+  ])("does not check skills when %s", async (_, arrange) => {
+    arrange();
+    const { checkSkillsForUpdate } = await import("./skillsUpdateCheck.js");
+    await checkSkillsForUpdate(true);
+    expect(mockCheckSkills).not.toHaveBeenCalled();
   });
 
   it("refreshSkillsCache persists the removed count alongside outdated/missing", async () => {

packages/cli/src/utils/skillsUpdateCheck.ts

@@ -7,6 +7,7 @@
 // the reminder on the commands they already run.
 
 import { readConfig, readConfigFresh, writeConfig } from "../telemetry/config.js";
+import { hostAnswers } from "./hostAnswers.js";
 import { checkSkills } from "./skillsManifest.js";
 import { updateNoticesSuppressed } from "./updateCheck.js";
 
@@ -101,15 +102,19 @@ export function invalidateSkillsCache(): void {
 }
 
 /**
- * Refresh the skills freshness cache if it is older than 24h. Best-effort:
+ * Refresh the skills freshness cache if it is older than 24h, only in runs that show the nudge. Best-effort:
  * any failure (offline, no manifest published yet, no skills installed) leaves
  * the cache untouched and reports "no update".
  *
  * @param force - skip the cache and check now
  */
 export async function checkSkillsForUpdate(force?: boolean): Promise<SkillsUpdateMeta> {
+  if (process.env["HYPERFRAMES_SKIP_SKILLS"] === "1" || updateNoticesSuppressed()) {
+    return getSkillsUpdateMeta();
+  }
   if (!force && cacheFresh(readConfig().lastSkillsCheck, Date.now())) return getSkillsUpdateMeta();
   try {
+    if (!(await hostAnswers("github.com"))) return getSkillsUpdateMeta();
     return await refreshSkillsCache();
   } catch {
     return getSkillsUpdateMeta();

packages/cli/src/utils/updateCheck.test.ts

@@ -1,6 +1,9 @@
 import { afterEach, describe, expect, it, vi } from "vitest";
 import { isSafeVersion, printDeprecationNotice, withMeta } from "./updateCheck.js";
 
+const dns = vi.hoisted(() => ({ answers: true }));
+vi.mock("./hostAnswers.js", () => ({ hostAnswers: async () => dns.answers }));
+
 describe("isSafeVersion", () => {
   it("accepts strict semver, incl. prerelease/build metadata", () => {
     expect(isSafeVersion("1.2.3")).toBe(true);
@@ -50,7 +53,9 @@ async function noticeWith(opts: {
     else process.env[k] = v;
   }
   // Default to a non-CI interactive terminal unless the test overrides env.
-  if (!("CI" in (opts.env ?? {}))) delete process.env["CI"];
+  for (const name of ["CI", "HYPERFRAMES_NO_UPDATE_CHECK"]) {
+    if (!(name in (opts.env ?? {}))) delete process.env[name];
+  }
 
   const origTTY = process.stderr.isTTY;
   Object.defineProperty(process.stderr, "isTTY", {
@@ -122,11 +127,16 @@ describe("printUpdateNotice — install-method-aware command", () => {
  * never be cached, because it flows into the auto-updater's install command.
  * This closes the injection class for every downstream consumer at one point.
  */
-async function checkWith(registryVersion: unknown): Promise<{
+async function checkWith(
+  registryVersion: unknown,
+  force = true,
+): Promise<{
   latest: string;
   wroteVersion: string | undefined;
+  fetched: boolean;
 }> {
   vi.resetModules();
+  vi.doMock("./env.js", () => ({ isDevMode: () => false }));
   const writes: Array<Record<string, unknown>> = [];
   vi.doMock("../telemetry/config.js", () => ({
     readConfig: () => ({}),
@@ -134,17 +144,19 @@ async function checkWith(registryVersion: unknown): Promise<{
     writeConfig: (c: Record<string, unknown>) => writes.push({ ...c }),
   }));
   const origFetch = globalThis.fetch;
-  globalThis.fetch = (async () => ({
-    ok: true,
-    json: async () => ({ version: registryVersion }),
-  })) as unknown as typeof fetch;
+  let fetched = false;
+  globalThis.fetch = (async () => {
+    fetched = true;
+    return { ok: true, json: async () => ({ version: registryVersion }) };
+  }) as unknown as typeof fetch;
   try {
     const mod = await import("./updateCheck.js");
-    const result = await mod.checkForUpdate(true);
+    const result = await mod.checkForUpdate(force);
     const lastWrite = writes.at(-1);
     return {
       latest: result.latest,
       wroteVersion: lastWrite ? (lastWrite["latestVersion"] as string | undefined) : undefined,
+      fetched,
     };
   } finally {
     globalThis.fetch = origFetch;
@@ -259,9 +271,53 @@ describe("printDeprecationNotice", () => {
 describe("checkForUpdate — registry boundary guard", () => {
   afterEach(() => {
     vi.doUnmock("../telemetry/config.js");
+    vi.doUnmock("./env.js");
     vi.resetModules();
   });
 
+  function clearOptOuts(): void {
+    vi.stubEnv("CI", "");
+    vi.stubEnv("HYPERFRAMES_NO_UPDATE_CHECK", "");
+  }
+
+  it("still asks the registry from a run without a terminal", async () => {
+    clearOptOuts();
+    const origTTY = process.stderr.isTTY;
+    Object.defineProperty(process.stderr, "isTTY", { value: false, configurable: true });
+    try {
+      expect((await checkWith("99.0.0", false)).fetched).toBe(true);
+    } finally {
+      Object.defineProperty(process.stderr, "isTTY", { value: origTTY, configurable: true });
+      vi.unstubAllEnvs();
+    }
+  });
+
+  it("skips the background registry request when DNS does not answer", async () => {
+    clearOptOuts();
+    dns.answers = false;
+    try {
+      expect((await checkWith("99.0.0", false)).fetched).toBe(false);
+      expect((await checkWith("99.0.0")).fetched).toBe(true);
+    } finally {
+      dns.answers = true;
+      vi.unstubAllEnvs();
+    }
+  });
+
+  it.each(["CI", "HYPERFRAMES_NO_UPDATE_CHECK"])(
+    "skips the background registry request when %s=1",
+    async (name) => {
+      clearOptOuts();
+      vi.stubEnv(name, "1");
+      try {
+        expect((await checkWith("99.0.0", false)).fetched).toBe(false);
+        expect((await checkWith("99.0.0")).fetched).toBe(true);
+      } finally {
+        vi.unstubAllEnvs();
+      }
+    },
+  );
+
   it("caches and returns a valid semver from the registry", async () => {
     const { latest, wroteVersion } = await checkWith("9.9.9");
     expect(latest).toBe("9.9.9");

packages/cli/src/utils/updateCheck.ts

@@ -4,6 +4,7 @@ import { compareVersions } from "compare-versions";
 import { readConfig, readConfigFresh, writeConfig } from "../telemetry/config.js";
 import { VERSION } from "../version.js";
 import { isDevMode } from "./env.js";
+import { hostAnswers } from "./hostAnswers.js";
 import { detectInstaller } from "./installerDetection.js";
 import { readPinnedHyperframesVersions } from "./projectPin.js";
 import { isSafeVersion } from "./safeVersion.js";
@@ -41,10 +42,11 @@ export interface UpdateMeta {
  * Check npm registry for the latest version. Uses a 24h cache to avoid
  * hitting the registry on every invocation.
  *
- * @param force - Skip cache and fetch fresh data
+ * @param force - Skip the cache, opt-outs and DNS probe: the caller waits for the registry
  */
 export async function checkForUpdate(force?: boolean): Promise<UpdateCheckResult> {
   const config = readConfig();
+  if (!force && updateCheckDisabled()) return fallbackResult(config.latestVersion);
   const now = Date.now();
 
   // Also guard the cache read: a cache written before this boundary guard
@@ -66,6 +68,9 @@ export async function checkForUpdate(force?: boolean): Promise<UpdateCheckResult
   }
 
   try {
+    if (!force && !(await hostAnswers(new URL(NPM_REGISTRY_URL).hostname))) {
+      return fallbackResult(config.latestVersion);
+    }
     const controller = new AbortController();
     const timeout = setTimeout(() => controller.abort(), FETCH_TIMEOUT_MS);
     const res = await fetch(NPM_REGISTRY_URL, {
@@ -156,17 +161,19 @@ export function printDeprecationNotice(command: string): void {
   );
 }
 
-/**
- * True when update / freshness notices should stay silent — CI, non-TTY, dev
- * mode, or the HYPERFRAMES_NO_UPDATE_CHECK opt-out. Shared with the skills
- * freshness notice so both honour the same gating.
- */
-export function updateNoticesSuppressed(): boolean {
+/** True when the update check is off: dev mode, CI, or the HYPERFRAMES_NO_UPDATE_CHECK opt-out. */
+export function updateCheckDisabled(): boolean {
   if (isDevMode()) return true;
   if (process.env["CI"] === "true" || process.env["CI"] === "1") return true;
-  if (!process.stderr.isTTY) return true;
-  if (process.env["HYPERFRAMES_NO_UPDATE_CHECK"] === "1") return true;
-  return false;
+  return process.env["HYPERFRAMES_NO_UPDATE_CHECK"] === "1";
+}
+
+/**
+ * True when update / freshness notices should stay silent: the check is off or stderr is not a
+ * terminal. Shared with the skills freshness notice so both honour the same gating.
+ */
+export function updateNoticesSuppressed(): boolean {
+  return updateCheckDisabled() || !process.stderr.isTTY;
 }
 
 /**