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

[miguel-heygen]

What

Three fixes around a catalog search that goes wrong.

1. A registry the network cannot reach no longer empties the catalog. An expired cache entry is now served when revalidation fails, instead of being discarded.
2. catalog --query hands back the gap-report command, pre-filled, and --json carries it as a new report_gap field.
3. Skill docs: /hyperframes-registry documents the gap channel and the stale-serve behaviour; /hyperframes-cli names three commands it never mentioned, and marks two that agents should not run.

Why

The cache fix. readCache returned undefined past its 24h TTL, which made "expired entry" and "no entry at all" indistinguishable to both callers. One timeout against the registry host therefore reported the entire catalog as unreachable, while a perfectly usable copy sat on disk. Reports from the field describe exactly this: a fetch that timed out on every item, and an author who hand-wrote captions, a chart and a wipe rather than install the blocks that already existed locally. The cost is silent, because an author who works around the catalog leaves no trace in it.

The gap report. feedback --search-miss exists to name moves the catalog does not have. It is the only signal that can describe a move nobody could install, which is precisely the thing install counts can never show. It was documented in a skill file and printed by feedback --help, and neither of those is open at the moment a query comes back wrong. The command was reachable only by an author who already remembered it existed.

The docs. /hyperframes-registry owns hyperframes catalog and never mentioned the gap channel, so the skill an agent actually loads for catalog work routed past it. Separately, a few CLI commands appear in --help and in no skill at all, and two more (events, and the deprecated check aliases) look useful but waste a turn.

How

registry/remote.ts — readCache becomes readCacheEntry + isFresh, so freshness is the caller's decision rather than a property of the read. Both fetchers keep their fast path (fresh entry, no network) and gain a fallback (stale entry once the fetch throws). One change, both call sites, since both routed through the same read.

Deliberately preserved:

• fetchRegistryManifest still returns undefined when the network fails and nothing was ever cached.
• fetchItemManifest still throws in that case, so callers can still tell "offline" from "no such item".
• skipCache forces revalidation but does not forbid the stale fallback. "Check for something newer" and "rather have nothing than this" are different requests, and only the first one is ever made.

commands/catalog.ts — a searchMissCommand(query, tier) helper builds the line with shell metacharacters escaped. It is printed on zero results, printed after an on-device search that returned hits, and carried in every --json search envelope. Only the command is built; sending it stays a separate deliberate act, so plain catalog --query keeps its promise that the query text never leaves the machine.

The nudge prints on both tiers, with whichever one answered. It was on-device-only at first, on the reasoning that a thin word-match result is explainable while a thin meaning-match result is a real gap. That reasoning was wrong in practice: the on-device tier needs a consented 33 MB download and is not even offered under --json, so an agent run ranks on words unless it passed --on-device --yes. Every catalog gap reported since the channel opened came from the word tier and none from on-device, so gating on on-device guaranteed the line would never reach the population that files these reports. The accompanying skill text went further and told agents not to report a word-tier miss at all, which would have sent an agent holding report_gap --tier words straight into discarding it.

The tier still rides along in the report, which is what keeps a vocabulary miss distinguishable from a meaning miss. That distinction belongs in the analysis, not in a gate that suppresses the data.

The --json field is unconditional on both tiers, because an agent judging the hits useless needs the command already in the envelope by the time it makes that call.

Test plan

• Unit tests added/updated
• Manual testing performed
• Documentation updated (if applicable)

packages/cli/src/registry/remote.test.ts is new: 8 tests covering the fresh path, the stale fallback on both fetchers, the stale fallback under skipCache, a successful refetch winning over the cache, and the two unchanged failure contracts. Verified that 3 of them fail against the unpatched remote.ts and the other 5 pass, so the suite is pinned to the behaviour change rather than to the implementation. The two stale tests assert the failing fetch was actually attempted, otherwise they would pass even if the clock never moved.

4 tests added to catalog.test.ts: report_gap present on a search with hits, the tier named correctly on the word tier, a CJK query surviving intact, and shell metacharacters escaped.

Exercised against the real CLI, both paths:

$ bun packages/cli/src/cli.ts catalog --query "zzzqqqxx nonexistent move" --tag no-such-tag
No items match query "zzzqqqxx nonexistent move" and tag "no-such-tag".

  Nothing in the catalog does this? Report the gap:
  npx hyperframes feedback --search-miss "zzzqqqxx nonexistent move" --wanted "<the move you needed>" --tier words

--json on a real query returns report_gap alongside tier and results.

Full CLI suite: 2632 passed. 3 failures remain (2 in transcribe, 1 hostname-dependent in telemetryIdentity); all 3 were confirmed failing on the base commit with this branch stashed, so they are pre-existing and untouched here.

Not covered

• fetchItemFile is unchanged. Item files are written straight to the destination and were never cached, so installing an item this machine has never seen still needs the network. Only discovery and re-install degrade gracefully.
• No telemetry for the degraded path. A stale-served registry is invisible; making it measurable touches the telemetry contract and belongs in its own change.
• Version skew is untouched. A stale npx cache can still run an older CLI than the version requested, which makes newer catalog flags look like they do not exist. That is not fixable from inside this repo and needs its own approach.
• Registry items that ship broken (blocks that fail our own lint/check as installed) are a separate seam: the fix there is gating the registry in CI with the linter we already have, not a change to the fetch path.

[jrusso1020]

The cache fix is the right shape and I'd approve it on the merits. I'm withholding the stamp for one reason: Typecheck is a required context, it is red at 306ab10, and this PR is what turned it red. Everything else below is either a second thing worth fixing while you're in there, or a note on what I checked so you know what this review does and does not cover.

Blocking: the new test files don't typecheck

Three errors, all in files this PR adds or edits:

src/commands/catalog.test.ts(358,21): error TS2339: Property 'report_gap' does not exist on type 'Envelope'.
src/commands/catalog.test.ts(371,21): error TS2339: Property 'report_gap' does not exist on type 'Envelope'.
src/registry/remote.test.ts(36,58):   error TS2344: Type '"fetch"' does not satisfy the constraint '"undefined" | "length" | ...'

Both are small:

• catalog.test.ts:173 — the local Envelope interface never got the new field. Adding report_gap: string; alongside shown closes it, and non-optional is the accurate model: both envelope-shaped emit sites are inside if (query) branches, and every one of them sets it.
• remote.test.ts:36 — the explicit return annotation Promise<ReturnType<typeof vi.spyOn<typeof globalThis, "fetch">>> is the only thing failing; the two vi.spyOn(globalThis, "fetch") calls on lines 37 and 42 typecheck fine, because there the key is inferred from the actual object rather than asserted against keyof typeof globalThis. Simplest fix is to drop the annotation and let the return type infer. If you want it written down, MockInstance<typeof fetch> from vitest says the same thing without the keyof constraint.

Worth naming the reason this got past a careful test plan: vitest transpiles without typechecking, so "2632 passed" and tsc are genuinely independent signals and the suite can be fully green while the package does not compile. bun run typecheck in packages/cli before the next push should be the whole of it.

The docs claim an offline capability the code does not have

skills/hyperframes-registry/SKILL.md adds:

A registry the CLI cannot reach does not empty the catalog: [...] so search and install still work against the last copy on disk. An item never fetched on this machine is the only thing a network failure can genuinely block.

The second sentence isn't right, and the PR body carries the same claim ("Only discovery and re-install degrade gracefully").

fetchItemFile is unchanged and item files are never cached — you say so yourself under "Not covered". Following it into installer.ts:152-160, the fetch is unconditional: the only branch that skips it is the local-edits guard (!force && existsSync(destPath) && hasLocalEdits(...)), which exists to protect a file you changed, not to serve one from disk. So a re-install of an item this machine installed yesterday still calls fetchItemFile for every file and still fails offline. What the cached item manifest buys is the file list, not the file contents — the install fails a step later, not less often.

So the accurate line is that discovery degrades gracefully and install does not. That distinction is worth getting exactly right in this file specifically, because it is the text an agent loads and acts on: as written it will tell someone to run add while offline, which fails at precisely the moment the PR is trying to make things better. The degenerate exception (every file locally edited, so every file is preserved and nothing is fetched) isn't worth documenting.

Nit: the truthiness test moved from the payload to the envelope

Narrow, and not what's blocking — but it is a behaviour change the diff introduces rather than a pre-existing wart, so it's worth a line.

Before, readCache returned entry.data and both callers tested the data: if (cached) return cached;. Now readCacheEntry returns the whole entry and the callers test the envelope: if (cached && isFresh(cached)) return cached.data;. Any parseable file with a numeric fetchedAt is truthy regardless of what data holds, so a cache file whose payload is null now short-circuits and returns null where it previously fell through to the network. In fetchItemManifest that hands back null typed as RegistryItem.

Reachable only if a registry ever serves a literal null body with a 200, or a cache file is hand-edited, so it's a hardening note rather than a bug I can demonstrate in the wild. One line in readCacheEntry covers it:

if (entry.data === undefined || entry.data === null) return undefined;

Fixing it also fails in the direction you care about here — toward "go ask the network" rather than toward "the catalog is empty".

What I checked and what held

• The cache mechanism, read against 67edb01. Splitting the read from the freshness decision is the correct fix for the described failure, and the three preserved contracts are genuinely preserved: fetchRegistryManifest still returns undefined with no cache, fetchItemManifest still throws, and skipCache still forces revalidation. Reading the cache file even under skipCache is a deliberate consequence of the stale fallback, not an oversight.
• "3 of 8 fail against the unpatched remote.ts" — traced, and it lands on exactly three. The stale-serve test on each fetcher plus the skipCache fallback flip; the other five pass either way. I verified this by reading both revisions rather than by executing the suite, so treat it as corroboration of your claim, not as an independent run.
• The stale tests assert the fetch was actually attempted (expect(fetchSpy).toHaveBeenCalledTimes(1)). That is the check that stops a fallback assertion from passing when the clock never moved, and it is usually the missing one. The mocked rejection not being a real AbortSignal.timeout doesn't weaken anything here, since the handler is a bare catch and cannot distinguish sources.
• report_gap coverage, by enumerating emit sites rather than trusting the summary. Three JSON.stringify sites in catalog.ts; the two query-bearing envelopes both carry the field, and the third is the query-less plain listing that the comment correctly leaves alone. "Every --json search envelope" checks out.
• The escaping actually pins its own regex flag. ["\\$]is exactly the set that stays special inside double quotes, and the test payload carries four distinct metacharacters plus a repeated"— so dropping the/g` and escaping only the first match fails the assertion. That is the row that usually slips through when a test reaches for a single hostile token. The command is built and printed, never handed to a shell, which is the right call.

Other CI signals, since neither is yours to fix

• Analyze (python) is red at this head and green at 67edb01, and the PR changes zero .py files. It is not in the required set. I'd re-run it rather than assume, but I see no mechanism by which this diff causes it.
• Tests on windows-latest is still pending and is required, so it owes a result regardless.

Scope: I read registry/remote.ts and commands/catalog.ts in full at both revisions, registry/remote.test.ts in full, the changed regions of catalog.test.ts, installer.ts around the install path, and all three docs files. I did not run the suite locally.

Happy to re-check quickly once the typecheck fixes are up — the substantive review above already carries, so it should just be confirming the three errors are gone.

— Rames Jusso

[miguel-heygen]

All three addressed in e922ff4. Thanks for tracing the install path rather than taking the PR body's word for it, that one was a real wrong claim and it was in the file that would have done the most damage.

Typecheck

Fixed, and bun run typecheck in packages/cli is clean now. Both diagnoses were exactly right:

• Envelope gained report_gap: string, non-optional for the reason you gave.
• The annotation is now MockInstance<typeof fetch>. Your read of why it failed while the vi.spyOn calls beside it didn't is correct.

Worth adding to your point about vitest transpiling without typechecking: the pre-commit typecheck hook does not cover packages/cli. It runs tsc in packages/core, packages/studio and scripts/ only. So this wasn't just me trusting a green suite over tsc; the local hook that exists to catch exactly this doesn't look at the package I changed, and it reported green. I've left that alone here since widening it is its own change with its own fallout, but it's the actual reason a careful test plan didn't save me and it will bite the next person the same way.

The offline claim

You're right and I've rewritten it. I followed installer.ts myself: the only branch that skips fetchItemFile is the local-edits guard, which exists to protect a file you edited, not to serve one from disk. So a re-install of yesterday's item still fetches every file and still fails offline. The cached manifest buys the file list, not the contents.

The skill now says discovery degrades gracefully and add does not, in those terms:

add still needs the network, even for an item you installed yesterday. Only manifests are cached; the item's actual files are fetched on every install. So offline you can search, and you can see what an item is, but installing it fails at the file fetch. Do not promise a user an offline install.

Your framing of why it matters in this file specifically is the part I got wrong when writing it, and it's now the reason the line is phrased as an instruction rather than a description.

The null payload

Taken, with a test. readCacheEntry now returns undefined for a null or undefined payload, and the new test primes the cache with a null body and asserts the next call goes to the network. I checked it fails with the guard removed rather than assuming it bites:

× fetchRegistryManifest > treats an empty cached payload as a miss rather than an answer

Agreed it's a hardening note rather than a demonstrable bug, but you're right that the truthiness target moved from the payload to the envelope in this diff, so it's mine to close.

Tests on windows-latest

Not blocking on my read, but it isn't obviously flake either, so here's what I actually checked rather than a guess.

Every failure in that job is a timeout or a spy-count assertion in files this PR does not touch:

• capture/contactSheet.test.ts — two tests at 60012ms and 60009ms
• utils/npxCommand.test.ts — 30013ms
• engine/services/audioMixer.test.ts — "expected spy to be called 3 times, but got 149 times"
• engine/utils/ffprobe.test.ts, engine/utils/urlDownloader.test.ts

My two files passed on that same Windows run: catalog.test.ts (27 tests) ✓ and remote.test.ts (8 tests) ✓.

Against that: you're right that Tests on windows-latest was green at 67edb01, so "pre-existing" isn't established and I'm not claiming it. The three round numbers (60012 / 60009 / 30013) read like a slow runner rather than a logic failure, and no mechanism connects a registry-cache change to ffprobe or the audio mixer. The push of e922ff4 gives a fresh run on the same unrelated files, which is the cheapest way to tell flake from signal. If it goes red again in the same places I'll dig in properly rather than re-run it a third time.

Analyze (python): agreed, zero .py files here and it isn't in the required set.