Add cursor pagination and cache validators to template registry - #765
Open
solaawojobi00-bit wants to merge 1 commit into
Open
Conversation
Adds stable cursor-based pagination and ETag/conditional-request caching to the template marketplace registry, closing Nanle-code#681. - templates::paginate()/Page<T>: opaque, base64-encoded cursors anchored to a template's name (not list position), so pages stay valid across calls even if the registry changes; rejects unknown cursors and a zero limit with clear errors. - fetch_and_cache_remote() now sends If-None-Match with the last-seen ETag and short-circuits on 304 Not Modified, reusing the local cache instead of re-downloading; the ETag is persisted in a sidecar file. - `starforge template list`/`search` gain --limit/--cursor (including the --json output path); omitting both keeps prior unpaginated behavior, so this is non-breaking. - registry_dir() honors STARFORGE_TEMPLATE_REGISTRY_DIR so tests don't depend on HOME, which dirs::home_dir() ignores on Windows. Also fixes three unrelated pre-existing compile errors blocking `cargo build` on master (missing thiserror dependency, a rusqlite::Transaction mutability bug in database.rs, and two missing `mod ai_doc_qa;` declarations), plus a bundled templates/registry.json field whose type didn't match the current TemplateEntry schema.
|
@solaawojobi00-bit Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
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.
Add cursor pagination and cache validators to the template registry
Problem
starforge template list/searchalways fetch and print the entire marketplace registry in one shot, and the local cache is refetched wholesale on every TTL expiry — there's no way to page through results, and no conditional-request support to avoid re-downloading an unchanged registry.Solution
templates::paginate()/Page<T>split a result set into pages of at most--limitentries. Cursors are opaque, base64-encoded, and anchored to a template's name rather than its list position, so a page stays valid even if entries are added elsewhere in the registry between calls. A cursor pointing at a since-removed entry is rejected with a clear error instead of silently skipping ahead.fetch_and_cache_remote()now sendsIf-None-Matchwith the last-seenETag; an unchanged registry replies304 Not Modifiedand the local cache is reused instead of re-downloading the body. The ETag is persisted in a sidecarregistry.etagfile.template list/searchwith neither--limitnor--cursorbehave exactly as before (show everything), so this is non-breaking.Changes
src/utils/templates.rspaginate(),Page<T>,encode_cursor/decode_cursor: stable, name-anchored cursor pagination over any slice.fetch_and_cache_remote()reworked to send a conditionalIf-None-Matchrequest and return aFetchOutcome::{Fetched, NotModified};load_registry()handles the304path by reusing the cache and resetting its TTL clock instead of rewriting it.registry_dir()now honorsSTARFORGE_TEMPLATE_REGISTRY_DIR(mirrors the existingSTARFORGE_TEMPLATE_REGISTRY_URLoverride) so tests don't depend onHOME—dirs::home_dir()ignoresHOME/USERPROFILEoverrides on Windows, and my first test run actually wrote fixtures into a real~/.starforge/templates/before I caught this and fixed it at the root.src/commands/template.rs--limit/--cursoradded toListandSearch(including the--jsonoutput path onlist, which now also reportsshown_count/next_cursor).print_pagination_footer()printsShown X of Yand the next cursor when a page was requested.templates/registry.json:security_review.findingswas stored as an integer (0,1) in several entries but the struct expectsOption<String>— parsing this bundled fallback registry failed outright. Caught by my own "fetch fails, fall back to bundled registry" test; fixed the data to match the schema.Cargo.toml/Cargo.lock,src/utils/database.rs,src/commands/mod.rs,src/utils/mod.rs: see Notes for Reviewers — three small, unrelated pre-existing bugs that were blockingcargo buildentirely onmasterbefore this PR could even be built against it.Regression Tests
paginate_walks_all_pages_in_orderpaginate_cursor_past_last_item_returns_empty_pagepaginate_rejects_zero_limit--limit 0paginate_rejects_malformed_cursor--cursorvaluepaginate_rejects_cursor_for_unknown_entryfetch_and_cache_remote_stores_and_sends_etag304load_registry_reuses_cache_on_304_after_forced_refresh304and reuses cacheload_registry_falls_back_to_bundled_default_when_remote_unreachableTesting
cargo build --lib --bin starforge— clean, no errors.cargo test --lib templates::(68 tests, run against a locally-patched build — see Notes for Reviewers on why):(64 other pre-existing
templates::tests unaffected and passing; one pre-existing, unrelated test —test_publish_template_versioned_stores_by_version— fails viafutures::executor::block_onnot providing a Tokio runtime context; not touched by this PR.)Manual CLI verification (
template list --limit N,--cursor,search --limit, invalid--cursor,--limit 0) was done earlier against an equivalent implementation before I discovered the branch/base issue described below; I was not able to re-run it interactively against this exact commit — see the stack-overflow note below.Notes for Reviewers
This PR ended up touching a few things beyond #681 itself, all pre-existing and discovered while getting a clean base to build against:
masterdoesn't currently build. Before any of my changes,cargo buildfails with a missingthiserrordependency (used via#[derive(thiserror::Error)]indatabase.rsbut never declared inCargo.toml) and arusqlite::Transactionmutability bug in the same file (Migration::up/downtook&mut Connection, butTransactiononly implementsDeref, notDerefMut, so&mut txnever actually worked — narrowed the trait to&Connection, which is all the one real implementation needs). Also, two modules (commands/ai_doc_qa.rs,utils/ai_doc_qa.rs) exist on disk but were never wired up viapub mod ai_doc_qa;, somain.rs's reference to them doesn't resolve. All three are fixed here since [2026 Registry] Add cursor pagination and cache validators #681 couldn't otherwise be built/tested at all.cargo test --libstill doesn't compile even after those fixes — 22 further pre-existing errors across ~10 unrelated files (plugins/manifest.rs,plugins/registry.rs,utils/compliance.rs,utils/bindings.rs,commands/audit.rs,utils/ai.rs,utils/template_analytics.rs,utils/template_recommender.rs), mostly test fixtures that fell out of sync with struct changes from other PRs. I did not fix these — out of scope for [2026 Registry] Add cursor pagination and cache validators #681 — but I did apply throwaway local patches to unblock compilation just long enough to run and confirm the tests listed above pass, then reverted those files before this commit. Worth a maintainer pass on its own.starforge --versioncrashes withSTATUS_STACK_OVERFLOWon Windows — reproduces identically with zero of this PR's changes present (confirmed by revertingtemplates.rs/commands/template.rs/registry.jsonand rebuilding), so it predates this PR. Best guess is Windows' 1MB default main-thread stack being exceeded by clap building the full ~90-module command tree at startup; very likely doesn't reproduce on Linux CI's 8MB default stack, but I couldn't confirm that (no WSL distro available in this environment) and couldn't fix it without a substantial, unrelated change (custom linker/thread-stack setup). Flagging so it isn't a surprise if CI's smoke-test job behaves differently than my local testing did.cargo test/clippy/smoke run on this branch — onlycargo buildand the pagination/caching-specific tests above are verified directly. Apologies for the scope of this note; happy to split thedatabase.rs/ai_doc_qafixes into a separate PR if preferred.Closes #681