Repository navigation
fix(registry): shield skills search route cache behind cached() - #251
Merged
Merged
Conversation
harlan-github-agent
Bot
added
harlan-agent-running
Contributor
Author
🤖 MERGED
GitHub merged this pull request. No material findings were recorded. 73a9f4d5-1b84-40d5-8507-6881d42e07f8 |
harlan-github-agent
Bot
added
harlan-agent-ready
harlan-github-agent
Bot
removed
the
harlan-agent-ready
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🔗 Linked issue
Closes #249
❓ Type of change
📚 Description
The daily-checkin routine caught a
KV GET failed: 500on/api/skills(Sentry SKILLD-28) and traced it to this route, the last registry route still on Nitro'sdefineCachedEventHandler. The detail route (#146) and skill-files (#201) hit the same family and moved to the repo's own SWR cache helper, where a failed KV read is a miss, a failed write is a dropped cache entry, neither can reach the request, and both report as wide events. This route now does the same, keeping the old cache identity: deployment-scoped, full sorted query string, hashed so distinct searches cannot collapse into one key.One caveat I measured before migrating: driving the installed Nitro 2.13.4 with a storage whose
getItemthrowsKV GET failed: 500 Internal Server Error, the request still answered 200. Nitro's own catch already absorbed the read and reported it throughcaptureError, which is what Sentry recorded. So on the current version the exposure was the shielding being a Nitro version detail plus a captured error per failed read, not a user-facing 500. The cache key namespace moved toskills-list:v2:because the stored envelope format changed; old entries age out within the 60s + 5min window.One thing for a reviewer to confirm: Nitro's route cache also set response headers (etag, last-modified,
x-nitro-cache); I believe nothing consumes those on this JSON surface, and #146 dropped them the same way, but I did not audit every client.