fix(catalog): survive an unreachable registry, and ask for the gap

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

@@ -1,6 +1,6 @@
 import { beforeEach, describe, expect, it, vi } from "vitest";
 
-import { countUnindexed, pickByName } from "./catalog.js";
+import { countUnindexed, pickByName, searchMissCommand } from "./catalog.js";
 
 /** The whole registry, which is what "in this registry" has to be measured against. */
 const registryNames = new Set(["fade-through", "whip-pan", "count-up"]);
@@ -177,6 +177,10 @@ interface Envelope {
   top_score?: number;
   shown: number;
   warnings?: string[];
+  // Not optional: both envelope-shaped emit sites are inside `if (query)`
+  // branches and both set it, so a search envelope without it is a bug rather
+  // than a shape the caller has to handle.
+  report_gap: string;
 }
 
 async function runCatalog(args: Record<string, unknown>): Promise<string> {
@@ -347,6 +351,45 @@ describe("catalog --json meaning search", () => {
       "on-device search is using the previous catalog vectors because the update failed",
     ]);
   });
+
+  it("hands back the gap-report command even when the search found things", async () => {
+    // The reports we actually want come from searches that returned plausible
+    // items where none of them did the job. If the command only appeared on
+    // zero results it would be absent from every case worth reporting.
+    const envelope = await runEnvelope({ query: "make a number count up" });
+
+    expect(envelope.shown).toBeGreaterThan(0);
+    expect(envelope.report_gap).toBe(
+      'npx hyperframes feedback --search-miss "make a number count up" ' +
+        '--wanted "<the move you needed>" --tier on-device',
+    );
+  });
+
+  it("names the tier that actually answered in the gap-report command", async () => {
+    state.modelStatus = "declined";
+    state.ranking = null;
+
+    const envelope = await runEnvelope({ query: "count up" });
+
+    expect(envelope.tier).toBe("words");
+    expect(envelope.report_gap).toContain("--tier words");
+  });
+});
+
+describe("searchMissCommand", () => {
+  it("keeps a non-ASCII query intact", () => {
+    // Half of the gap reports received so far were CJK. A query mangled on the
+    // way into the command is a report nobody can act on.
+    expect(searchMissCommand("実写写真のみ 9:16 生活ハック", "on-device")).toContain(
+      '--search-miss "実写写真のみ 9:16 生活ハック"',
+    );
+  });
+
+  it("escapes shell metacharacters so the printed line is safe to paste", () => {
+    const cmd = searchMissCommand('a "quoted" $VAR `sub` \\ thing', "words");
+
+    expect(cmd).toContain('--search-miss "a \\"quoted\\" \\$VAR \\`sub\\` \\\\ thing"');
+  });
 });
 
 describe("catalog meaning search, on a terminal", () => {
@@ -364,6 +407,26 @@ describe("catalog meaning search, on a terminal", () => {
 
     expect(output).not.toContain("missing from the on-device index");
   });
+
+  it("offers the gap report on the word tier, not just on-device", async () => {
+    // The tier that answers almost every real search, because on-device needs
+    // a consented download. Gating the nudge on on-device left it unprinted in
+    // the only case that occurs, which is how the gap channel stayed silent.
+    state.modelStatus = "declined";
+    state.ranking = null;
+
+    const output = await runCatalog({ query: "count up" });
+
+    expect(output).toContain("None of these do it?");
+    expect(output).toContain("--tier words");
+  });
+
+  it("offers the gap report on the on-device tier too", async () => {
+    const output = await runCatalog({ query: "make a number count up" });
+
+    expect(output).toContain("None of these do it?");
+    expect(output).toContain("--tier on-device");
+  });
 });
 
 describe("the on-device download offer", () => {

packages/cli/src/commands/catalog.ts

@@ -259,6 +259,7 @@ export default defineCommand({
               shown: 0,
               total: tagged.length,
               ...(warnings.length ? { warnings } : {}),
+              report_gap: searchMissCommand(query, tierToken(searched)),
               results: [],
             },
             null,
@@ -274,6 +275,13 @@ export default defineCommand({
           args.tag ? `tag "${args.tag}"` : null,
         ].filter(Boolean);
         console.log(`No items match ${criteria.join(" and ")}.`);
+        // Zero results is the unambiguous case: no tier judgement to make and
+        // nothing to install, so name the gap channel outright.
+        if (query) {
+          console.log("");
+          console.log(c.dim("  Nothing in the catalog does this? Report the gap:"));
+          console.log(c.dim(`  ${searchMissCommand(query, tierToken(searched))}`));
+        }
       }
       if (query) await offerLocalModel(0, json, config.registry, artifactRevision);
       return;
@@ -307,6 +315,12 @@ export default defineCommand({
             shown: output.length,
             total: tagged.length,
             ...(warnings.length ? { warnings } : {}),
+            // Carried on hits too, not just on zero results. The gaps that
+            // actually get reported are the ones where the search returned
+            // plausible items and none of them did the thing — a judgement
+            // only the reader can make, so the command has to already be in
+            // the envelope by the time they make it.
+            report_gap: searchMissCommand(query, tierToken(searched)),
             results: output,
           },
           null,
@@ -350,6 +364,19 @@ export default defineCommand({
           await offerLocalModel(matching.length, json, config.registry, artifactRevision);
         }
       }
+      if (query) {
+        // Both tiers, deliberately. Gating this on on-device sounded right --
+        // a thin word-match result is explainable, a thin meaning-match result
+        // is a real gap -- but it silences the line in the case that produces
+        // essentially every search: the on-device tier needs a consented 33 MB
+        // download, so an agent run is on `words` unless it explicitly opted
+        // in. Every catalog gap reported to date came from the word tier. The
+        // tier rides along in the command so a vocabulary miss stays
+        // distinguishable from a meaning miss when the reports are read.
+        console.log(
+          c.dim(`  None of these do it? ${searchMissCommand(query, tierToken(searched))}`),
+        );
+      }
     }
 
     if (interactive) {
@@ -559,6 +586,31 @@ function tierDetail(searched: { localMode: LocalMode } | null): string {
   return searched?.localMode === "local-model" ? "on-device meaning search" : "local word match";
 }
 
+/**
+ * The command that turns a fruitless search into a catalog gap report.
+ *
+ * A search that returns nothing usable is the only moment anyone knows what
+ * the catalog is missing, and it was also the one moment we said nothing:
+ * `feedback --search-miss` was documented in the skill and printed by
+ * `feedback --help`, neither of which is open when a query comes back wrong.
+ * Handing back the exact line, with the query already in it, is the whole
+ * difference between a gap someone reports and a gap someone works around.
+ *
+ * Only the command is built here. Sending it stays a separate deliberate act,
+ * so plain `catalog --query` keeps its promise that the query text never
+ * leaves the machine.
+ */
+export function searchMissCommand(query: string, tier: "on-device" | "words"): string {
+  // Double quotes with the shell metacharacters escaped: the queries that
+  // matter are plain-language phrases, and half the real ones so far were
+  // CJK, which single-quoting renders no more safely and reads worse.
+  const quoted = query.replace(/(["\\$`])/g, "\\$1");
+  return (
+    `npx hyperframes feedback --search-miss "${quoted}" ` +
+    `--wanted "<the move you needed>" --tier ${tier}`
+  );
+}
+
 type LocalMode = "local-model" | "words";
 
 /**

packages/cli/src/registry/remote.test.ts

@@ -0,0 +1,162 @@
+import { afterAll, afterEach, beforeEach, describe, expect, it, vi } from "vitest";
+import type { MockInstance } from "vitest";
+import { mkdtempSync, rmSync } from "node:fs";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
+
+// The cache lives under homedir(), so the whole suite runs against a scratch
+// home rather than the developer's own ~/.hyperframes.
+const scratchHome = mkdtempSync(join(tmpdir(), "hf-remote-"));
+vi.mock("node:os", async (importOriginal) => ({
+  ...(await importOriginal<typeof import("node:os")>()),
+  homedir: () => scratchHome,
+}));
+
+const { fetchItemManifest, fetchRegistryManifest, DEFAULT_REGISTRY_URL } =
+  await import("./remote.js");
+
+const MANIFEST = { name: "hyperframes", items: [{ name: "count-up" }] };
+const ITEM = { name: "count-up", type: "hyperframes:component", files: [] };
+
+const ONE_DAY_MS = 24 * 60 * 60 * 1000;
+
+function ok(body: unknown): Response {
+  return { ok: true, status: 200, json: async () => body } as unknown as Response;
+}
+
+/**
+ * Prime the cache with one good fetch, then jump past the 24h TTL with every
+ * later fetch failing: the exact shape of "the registry host stopped answering
+ * and the copy on disk is a day old". Returns the failing spy so a caller can
+ * assert the network was actually attempted — without that the fallback
+ * assertions pass even if the clock never moved.
+ */
+async function staleAfterPriming(
+  body: unknown,
+  prime: () => Promise<unknown>,
+): Promise<MockInstance<typeof fetch>> {
+  vi.spyOn(globalThis, "fetch").mockResolvedValueOnce(ok(body));
+  await prime();
+
+  vi.useFakeTimers({ shouldAdvanceTime: true });
+  vi.setSystemTime(new Date(Date.now() + ONE_DAY_MS + 60_000));
+  return vi.spyOn(globalThis, "fetch").mockRejectedValue(new Error("The operation was aborted"));
+}
+
+beforeEach(() => {
+  rmSync(join(scratchHome, ".hyperframes"), { recursive: true, force: true });
+  vi.restoreAllMocks();
+  vi.useRealTimers();
+});
+
+afterEach(() => {
+  vi.useRealTimers();
+});
+
+afterAll(() => {
+  rmSync(scratchHome, { recursive: true, force: true });
+});
+
+describe("fetchRegistryManifest", () => {
+  it("serves a fresh cache without touching the network", async () => {
+    const fetchSpy = vi.spyOn(globalThis, "fetch").mockResolvedValue(ok(MANIFEST));
+    await fetchRegistryManifest(DEFAULT_REGISTRY_URL);
+    expect(fetchSpy).toHaveBeenCalledTimes(1);
+
+    const second = await fetchRegistryManifest(DEFAULT_REGISTRY_URL);
+
+    expect(second).toEqual(MANIFEST);
+    expect(fetchSpy).toHaveBeenCalledTimes(1);
+  });
+
+  it("serves the expired cache when the registry host times out", async () => {
+    // The failure this exists for: raw.githubusercontent.com stops answering,
+    // the entry is a day and a bit old, and before this fix the caller was
+    // told the whole catalog was unreachable while a usable copy sat on disk.
+    const fetchSpy = await staleAfterPriming(MANIFEST, () =>
+      fetchRegistryManifest(DEFAULT_REGISTRY_URL),
+    );
+
+    await expect(fetchRegistryManifest(DEFAULT_REGISTRY_URL)).resolves.toEqual(MANIFEST);
+    expect(fetchSpy).toHaveBeenCalledTimes(1);
+  });
+
+  it("treats an empty cached payload as a miss rather than an answer", async () => {
+    // The callers test the entry, not the payload, so a file carrying a valid
+    // fetchedAt and a null body would otherwise short-circuit the fetch and be
+    // handed back as a manifest.
+    vi.spyOn(globalThis, "fetch").mockResolvedValueOnce(ok(null));
+    await fetchRegistryManifest(DEFAULT_REGISTRY_URL);
+
+    const fetchSpy = vi.spyOn(globalThis, "fetch").mockResolvedValue(ok(MANIFEST));
+
+    await expect(fetchRegistryManifest(DEFAULT_REGISTRY_URL)).resolves.toEqual(MANIFEST);
+    expect(fetchSpy).toHaveBeenCalledTimes(1);
+  });
+
+  it("still reports unreachable when the network fails and nothing was cached", async () => {
+    vi.spyOn(globalThis, "fetch").mockRejectedValue(new Error("getaddrinfo ENOTFOUND"));
+
+    await expect(fetchRegistryManifest(DEFAULT_REGISTRY_URL)).resolves.toBeUndefined();
+  });
+
+  it("falls back to the stale copy under skipCache too", async () => {
+    // skipCache asks for something newer. It has never meant "rather have
+    // nothing than this", so a failed revalidation must not empty the result.
+    vi.spyOn(globalThis, "fetch").mockResolvedValueOnce(ok(MANIFEST));
+    await fetchRegistryManifest(DEFAULT_REGISTRY_URL);
+
+    vi.spyOn(globalThis, "fetch").mockRejectedValue(new Error("HTTP 503"));
+
+    await expect(fetchRegistryManifest(DEFAULT_REGISTRY_URL, { skipCache: true })).resolves.toEqual(
+      MANIFEST,
+    );
+  });
+
+  it("prefers a successful refetch over the cached copy", async () => {
+    vi.spyOn(globalThis, "fetch").mockResolvedValueOnce(ok(MANIFEST));
+    await fetchRegistryManifest(DEFAULT_REGISTRY_URL);
+
+    const fresher = { ...MANIFEST, items: [{ name: "count-up" }, { name: "push-in" }] };
+    vi.spyOn(globalThis, "fetch").mockResolvedValue(ok(fresher));
+
+    await expect(fetchRegistryManifest(DEFAULT_REGISTRY_URL, { skipCache: true })).resolves.toEqual(
+      fresher,
+    );
+  });
+});
+
+describe("fetchItemManifest", () => {
+  it("serves the expired cache when the item fetch fails", async () => {
+    const fetchSpy = await staleAfterPriming(ITEM, () =>
+      fetchItemManifest("count-up", "hyperframes:component", DEFAULT_REGISTRY_URL),
+    );
+
+    await expect(
+      fetchItemManifest("count-up", "hyperframes:component", DEFAULT_REGISTRY_URL),
+    ).resolves.toEqual(ITEM);
+    expect(fetchSpy).toHaveBeenCalledTimes(1);
+  });
+
+  it("still throws when the item fetch fails and nothing was cached", async () => {
+    // The documented contract for a genuinely unknown item, unchanged: callers
+    // that install by name have to be able to tell "offline" from "no such item".
+    vi.spyOn(globalThis, "fetch").mockRejectedValue(new Error("The operation was aborted"));
+
+    await expect(
+      fetchItemManifest("never-fetched", "hyperframes:component", DEFAULT_REGISTRY_URL),
+    ).rejects.toThrow("The operation was aborted");
+  });
+
+  it("surfaces an HTTP error for an item that does not exist", async () => {
+    vi.spyOn(globalThis, "fetch").mockResolvedValue({
+      ok: false,
+      status: 404,
+      json: async () => ({}),
+    } as unknown as Response);
+
+    await expect(
+      fetchItemManifest("no-such-move", "hyperframes:component", DEFAULT_REGISTRY_URL),
+    ).rejects.toThrow("HTTP 404");
+  });
+});