diff --git a/hub/CHANGELOG.md b/hub/CHANGELOG.md index b649e93..b4ca1f7 100644 --- a/hub/CHANGELOG.md +++ b/hub/CHANGELOG.md @@ -1,3 +1,34 @@ +## v0.101.0 — the artifact dropdown is memoised for 60 seconds (2026-08-08, R-267, operator ruling) + +v0.100.x took `/configuration` from 26.2 s to a mean of ~9.85 s by removing the serialisation. What +remained was **one Gitea call per dropdown** — the package search — measured at **0.20–3.8 s each +depending on load**, which concurrency cannot help (two together took 3.68 s against 3.82 s for one +alone; Gitea appears to serialise them). + +**Memoised for 60 s, in memory.** The operator ruled the TTL against the one workflow that cares: a +bake-and-vouch session publishes an artifact and comes straight here to select it. A minute is short +enough not to be noticed and long enough that every reload in that session is instant. + +**NOT persisted, and the reason is a rule this project already holds.** Gitea IS the store for both +the version list and the sha. A copy in `hub_settings` would be a second source of truth that can +drift from the registry it describes — and the operator reads the sha here to confirm what they are +about to vouch, so a stale one would be a confident wrong answer. `golden_currency_gate.py` records +the same reasoning for the vouched version. An in-memory cache dies with the process and can never be +mistaken for a record. + +**A failed resolve is not cached** — a Gitea blip must not pin an empty dropdown in front of the +operator for a minute. **But an empty list from a package that genuinely has no versions IS cached**, +because "we found nothing" and "we could not look" are different answers; that distinction is +CONTEXT S-39 applied to a list instead of a figure. **The first version of this commit got that +wrong** — the comment claimed only successful resolves were cached and the code cached the empty +list anyway. `TestArtifactChoices_FailureIsNotCached` caught it before it shipped, which is the whole +argument for writing the test that asserts the comment. + +Seven tests now cover the dropdown: order (and that each sha belongs to its own version), +per-version failure isolation, the concurrency itself (wall-clock plus an in-flight counter plus an +upper bound), the cap, the TTL hit, the failure-not-cached rule, and per-package isolation. +`go test -race` clean. + ## v0.100.2 — the Configuration page took 26 seconds, and it was never hashing anything (2026-08-08) **Measured, not guessed:** `GET /configuration` → **HTTP 200 in 26.2 s**. diff --git a/hub/internal/web/artifact_choices_test.go b/hub/internal/web/artifact_choices_test.go index 8bdea71..b4459db 100644 --- a/hub/internal/web/artifact_choices_test.go +++ b/hub/internal/web/artifact_choices_test.go @@ -192,3 +192,57 @@ func TestArtifactChoices_StillCapsAtTwenty(t *testing.T) { atomic.LoadInt32(&f.callCount)) } } + +// R-267: the dropdown is memoised for artifactChoicesTTL. A second load inside the window must make +// NO further Gitea calls; a failed resolve must NOT be cached, or a blip would pin an empty list in +// front of the operator for a minute. +func TestArtifactChoices_CachedWithinTTL(t *testing.T) { + f := &fakeGitea{versions: []string{"0.9.0", "0.8.0"}} + s := newTestWebServer(t, f.start(t)) + + first := s.artifactChoices(context.Background(), "felhom-agent", "felhom-agent") + after := atomic.LoadInt32(&f.callCount) + if after == 0 { + t.Fatal("the first resolve made no calls") + } + second := s.artifactChoices(context.Background(), "felhom-agent", "felhom-agent") + + if atomic.LoadInt32(&f.callCount) != after { + t.Errorf("a second load inside the TTL hit Gitea again (%d -> %d calls) — the cache is not "+ + "in the path", after, atomic.LoadInt32(&f.callCount)) + } + if len(second) != len(first) || second[0].Version != first[0].Version || second[0].SHA256 != first[0].SHA256 { + t.Errorf("cached result differs from the fresh one: %+v vs %+v", second, first) + } +} + +func TestArtifactChoices_FailureIsNotCached(t *testing.T) { + // every version fails → an empty list, which must NOT be remembered + f := &fakeGitea{versions: []string{"0.9.0"}, failVer: "0.9.0"} + s := newTestWebServer(t, f.start(t)) + + if got := s.artifactChoices(context.Background(), "felhom-agent", "felhom-agent"); len(got) != 0 { + t.Fatalf("precondition: expected an empty list, got %+v", got) + } + before := atomic.LoadInt32(&f.callCount) + _ = s.artifactChoices(context.Background(), "felhom-agent", "felhom-agent") + if atomic.LoadInt32(&f.callCount) == before { + t.Error("an empty/failed resolve was cached — a Gitea blip would then show the operator an " + + "empty dropdown for a full minute with no way to retry") + } +} + +// Two packages must not share a cache slot. +func TestArtifactChoices_CacheIsPerPackage(t *testing.T) { + f := &fakeGitea{versions: []string{"0.9.0"}} + s := newTestWebServer(t, f.start(t)) + a := s.artifactChoices(context.Background(), "felhom-agent", "felhom-agent") + before := atomic.LoadInt32(&f.callCount) + _ = s.artifactChoices(context.Background(), "felhom-golden", "golden.tar.zst") + if atomic.LoadInt32(&f.callCount) == before { + t.Error("a different package was served from the first package's cache entry") + } + if len(a) == 0 { + t.Error("precondition") + } +} diff --git a/hub/internal/web/server.go b/hub/internal/web/server.go index 8cf96c5..247417d 100644 --- a/hub/internal/web/server.go +++ b/hub/internal/web/server.go @@ -88,6 +88,11 @@ type Server struct { sessions map[string]*hubSession sessionsMu sync.RWMutex + + // artifactCache (R-267) memoises the Day-0 dropdown contents for artifactChoicesTTL. In memory + // only — see the constant's comment for why this must never become a database row. + artifactCache map[string]artifactChoiceCacheEntry + artifactCacheMu sync.Mutex } // New creates a new web server. @@ -223,10 +228,37 @@ func (s *Server) SetGiteaClient(c *gitea.Client) { // from Gitea, newest first, for the Day-0 artifact dropdown. Returns nil (→ manual text-entry // fallback) when no Gitea client is configured or Gitea is unreachable. A failed sha lookup for a // single version drops just that version, not the whole list. +// artifactChoicesTTL bounds how stale the dropdown may be (R-267, operator ruling 2026-08-08). +// +// 60 s, chosen deliberately against the one workflow that cares: a bake-and-vouch session publishes +// an artifact and then goes straight to this page to select it. A minute is short enough that the +// operator does not notice waiting for it, and long enough that the page is instant for every reload +// during that session. A longer TTL was rejected for exactly that case — it would hide a +// just-published golden at the moment someone is looking for it. +// +// NOT persisted, and that is the point. Gitea IS the store for both the version list and the sha; +// a copy in the hub's database would be a second source of truth that can drift from the registry +// it describes, and the operator reads the sha here to confirm what they are about to vouch. An +// in-memory cache dies with the process and can never be mistaken for a record. +const artifactChoicesTTL = 60 * time.Second + +type artifactChoiceCacheEntry struct { + choices []artifactChoice + at time.Time +} + func (s *Server) artifactChoices(ctx context.Context, pkg, file string) []artifactChoice { if s.gitea == nil { return nil } + + s.artifactCacheMu.Lock() + if e, ok := s.artifactCache[pkg]; ok && time.Since(e.at) < artifactChoicesTTL { + s.artifactCacheMu.Unlock() + return e.choices + } + s.artifactCacheMu.Unlock() + vers, err := s.gitea.ListVersions(ctx, pkg) if err != nil { s.logger.Printf("[WARN] artifact versions (%s): %v", pkg, err) @@ -277,6 +309,25 @@ func (s *Server) artifactChoices(ctx context.Context, pkg, file string) []artifa } out = append(out, artifactChoice{Version: v, SHA256: shas[i]}) } + + // Only a SUCCESSFUL resolve is cached. A Gitea blip must not pin an empty list in front of the + // operator for a minute — the earlier returns leave the cache untouched, and so does this one + // when every version's sha lookup failed. + // + // The `len(vers) > 0` half matters: a package that genuinely has no versions yet resolves to an + // empty list legitimately, and caching THAT is correct. Distinguishing the two is the whole + // point — "we found nothing" and "we could not look" are not the same answer, which is the rule + // CONTEXT S-39 states for figures and applies just as well here. + if len(out) == 0 && len(vers) > 0 { + s.logger.Printf("[WARN] artifact choices (%s): every version's sha lookup failed — not caching", pkg) + return out + } + s.artifactCacheMu.Lock() + if s.artifactCache == nil { + s.artifactCache = map[string]artifactChoiceCacheEntry{} + } + s.artifactCache[pkg] = artifactChoiceCacheEntry{choices: out, at: time.Now()} + s.artifactCacheMu.Unlock() return out }