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

Serve an expired registry cache when revalidation fails, so one timeout against the registry host no longer reports the whole catalog as unreachable while a usable copy sits on disk.

Hand back the gap-report command at the moment a search comes back wrong: catalog --query prints it pre-filled on both tiers, and every --json search envelope carries it as report_gap. Report on either tier, since the on-device tier needs a consented download and every gap reported to date came from the word tier.

Document the gap channel in the registry skill, which owns hyperframes catalog and never mentioned it, and name the CLI commands no skill did.
This commit is contained in:
Miguel Ángel
2026-08-17 14:55:17 -04:00
committed by GitHub
parent 67edb01bf4
commit 37f8c48449
7 changed files with 355 additions and 22 deletions
+162
View File
@@ -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");
});
});
+46 -17
View File
@@ -45,18 +45,36 @@ function cachePath(baseUrl: string, key: string): string {
return join(CACHE_DIR, `${slug}__${key}.json`);
}
function readCache<T>(path: string): T | undefined {
/**
* Read a cache entry regardless of age. Freshness is deliberately NOT decided
* here: a fresh entry lets a caller skip the network, and a stale one is still
* the best answer available once the network has already failed. Collapsing
* the two (returning undefined past the TTL) made a 25-hour-old manifest and
* no manifest at all indistinguishable, so one timeout against the registry
* host reported the entire catalog as unreachable and sent authors off to
* hand-write what they already had on disk.
*/
function readCacheEntry<T>(path: string): CacheEntry<T> | undefined {
try {
const entry = JSON.parse(readFileSync(path, "utf-8")) as CacheEntry<T>;
if (typeof entry.fetchedAt !== "number") return undefined;
if (Date.now() - entry.fetchedAt > CACHE_TTL_MS) return undefined;
return entry.data;
// The callers now test the entry rather than the payload, so an empty
// payload would satisfy them: `null` data would short-circuit the fetch
// and be handed back as a RegistryItem. Rejecting it here keeps the miss
// failing toward "go ask the network" rather than toward "the catalog is
// empty", which is the whole point of the change around it.
if (entry.data === undefined || entry.data === null) return undefined;
return entry;
} catch {
// Missing file or corrupt JSON → cache miss.
return undefined;
}
}
function isFresh<T>(entry: CacheEntry<T>): boolean {
return Date.now() - entry.fetchedAt <= CACHE_TTL_MS;
}
function writeCache<T>(path: string, data: T): void {
try {
mkdirSync(dirname(path), { recursive: true });
@@ -79,31 +97,37 @@ async function fetchJson<T>(url: string): Promise<T> {
}
/**
* Fetch the top-level registry.json manifest. Cached for 24h.
* Returns undefined if the registry is unreachable (offline / 404).
* Fetch the top-level registry.json manifest. Served from cache while fresh,
* revalidated after 24h, and — when revalidation fails — served stale rather
* than not at all. Returns undefined only when the registry is unreachable AND
* nothing was ever cached.
*
* `skipCache` forces revalidation; it does not forbid the stale fallback,
* because "check for something newer" and "rather have nothing than this" are
* different requests and only the first one is ever made.
*/
export async function fetchRegistryManifest(
baseUrl: string = DEFAULT_REGISTRY_URL,
options?: { skipCache?: boolean },
): Promise<RegistryManifest | undefined> {
const cacheFile = cachePath(baseUrl, "registry");
if (!options?.skipCache) {
const cached = readCache<RegistryManifest>(cacheFile);
if (cached) return cached;
}
const cached = readCacheEntry<RegistryManifest>(cacheFile);
if (!options?.skipCache && cached && isFresh(cached)) return cached.data;
try {
const manifest = await fetchJson<RegistryManifest>(`${baseUrl}/registry.json`);
writeCache(cacheFile, manifest);
return manifest;
} catch {
return undefined;
return cached?.data;
}
}
/**
* Fetch a single item's `registry-item.json` manifest. Cached for 24h.
* Throws on network failure (callers decide whether to degrade gracefully).
* Fetch a single item's `registry-item.json` manifest. Same freshness policy as
* the top-level manifest: fresh from cache, else revalidate, else serve stale.
* Throws on network failure only when nothing was ever cached for this item
* (callers decide whether to degrade gracefully).
*/
export async function fetchItemManifest(
name: string,
@@ -112,13 +136,18 @@ export async function fetchItemManifest(
): Promise<RegistryItem> {
const dir = ITEM_TYPE_DIRS[type];
const cacheFile = cachePath(baseUrl, `${dir}__${name}`);
const cached = readCache<RegistryItem>(cacheFile);
if (cached) return cached;
const cached = readCacheEntry<RegistryItem>(cacheFile);
if (cached && isFresh(cached)) return cached.data;
const url = `${baseUrl}/${dir}/${name}/registry-item.json`;
const item = await fetchJson<RegistryItem>(url);
writeCache(cacheFile, item);
return item;
try {
const item = await fetchJson<RegistryItem>(url);
writeCache(cacheFile, item);
return item;
} catch (err) {
if (cached) return cached.data;
throw err;
}
}
/**