Answer the review; render the front page's sky from estilo, with its placeholder
- If-None-Match uses weak comparison (RFC 9110): whatever gzips oficina.works turns the strong tag into W/"...", which an exact compare would never match, so nothing would ever be a 304. - Pin estilo 4d41ba2 (the sky's instant placeholder). The front page no longer hard-codes the sky's images: a marker is replaced on first serve by estilo's own template, placeholder included, so the page can never drift from the estilo it pins. - An unused helper removed.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
9 files changed,
+82
−30
go.mod+1 −1go.sum+2 −0internal/home/assetversion.go+14 −7internal/home/assetversion_test.go+18 −0internal/home/origins_test.go+5 −1internal/home/page.go+18 −1internal/home/page_test.go+22 −12internal/home/passkey.go+1 −1internal/home/static/site/index.html+1 −7
CI passed — run details
CI log
amadan: .amadan/timeout: this job's budget is 15m0s (this runner allows up to 30m0s) === vet === CGO_ENABLED=0 go vet ./... go: downloading amadan.net/oficina/estilo.git v0.0.0-20260929082849-4d41ba2100ff cd handoff && CGO_ENABLED=0 go vet ./... cd roster && CGO_ENABLED=0 go vet ./... cd connect && CGO_ENABLED=0 go vet ./... === fmt === === build === CGO_ENABLED=0 go build ./... CGO_ENABLED=0 GOOS=linux GOARCH=arm64 go build ./cmd/home ./cmd/oficina ./internal/operatorlaunch cd handoff && CGO_ENABLED=0 go build ./... cd roster && CGO_ENABLED=0 go build ./... cd connect && CGO_ENABLED=0 go build ./... === go-test === CGO_ENABLED=0 go test -count=1 ./... ? amadan.net/oficina/home/cmd/home [no test files] ok amadan.net/oficina/home/cmd/home-placeholder 0.883s ? amadan.net/oficina/home/cmd/oficina [no test files] ok amadan.net/oficina/home/internal/guardstest 0.052s ok amadan.net/oficina/home/internal/home 14.654s ok amadan.net/oficina/home/internal/home/directorydata 1.755s ok amadan.net/oficina/home/internal/home/logtest 0.006s ok amadan.net/oficina/home/internal/home/seal 0.014s ok amadan.net/oficina/home/internal/home/sealtest 0.005s ok amadan.net/oficina/home/internal/operator 5.973s ok amadan.net/oficina/home/internal/operatorlaunch 4.240s ok amadan.net/oficina/home/internal/provisioner 44.375s cd handoff && CGO_ENABLED=0 go test -count=1 ./... ok amadan.net/oficina/home/handoff 0.321s ok amadan.net/oficina/home/handoff/seal 0.002s cd roster && CGO_ENABLED=0 go test -count=1 ./... ok amadan.net/oficina/home/roster 5.086s cd connect && CGO_ENABLED=0 go test -count=1 ./... ok amadan.net/oficina/home/connect 0.125s ok amadan.net/oficina/home/connect/linkstore 0.385s === race === go test -race -count=1 ./internal/home/ ./internal/operator/ ./internal/operatorlaunch/ ./internal/provisioner/ ./internal/home/directorydata/ ok amadan.net/oficina/home/internal/home 104.905s ok amadan.net/oficina/home/internal/operator 64.908s ok amadan.net/oficina/home/internal/operatorlaunch 22.364s ok amadan.net/oficina/home/internal/provisioner 299.893s ok amadan.net/oficina/home/internal/home/directorydata 15.315s cd handoff && CGO_ENABLED=1 go test -race -count=1 ./... ok amadan.net/oficina/home/handoff 10.173s ok amadan.net/oficina/home/handoff/seal 1.011s cd roster && CGO_ENABLED=1 go test -race -count=1 ./... ok amadan.net/oficina/home/roster 6.652s cd connect && CGO_ENABLED=1 go test -race -count=1 ./... ok amadan.net/oficina/home/connect 1.629s ok amadan.net/oficina/home/connect/linkstore 3.759s
diff --git a/go.mod b/go.mod| index 7823eeb..0f8db18 100644 |
| --- a/go.mod |
| +++ b/go.mod |
| @@ -5,7 +5,7 @@ go 1.25.0 |
| toolchain go1.26.5 |
| require ( |
| - amadan.net/oficina/estilo.git v0.0.0-20260929070530-5b2ce835e773 |
| + amadan.net/oficina/estilo.git v0.0.0-20260929082849-4d41ba2100ff |
| amadan.net/oficina/home/handoff v0.0.0-20260908102137-481d2d57f489 |
| amadan.net/oficina/home/roster v0.0.0-20260908123429-74dda570e0cf |
| amadan.net/rastrillo/rastrillo v0.26.1-0.20260908075707-29e4491631bd |
diff --git a/go.sum b/go.sum| index c3fdab5..b2b0844 100644 |
| --- a/go.sum |
| +++ b/go.sum |
| @@ -10,6 +10,8 @@ amadan.net/oficina/estilo.git v0.0.0-20260929070257-7a1a376ad2ab h1:ypKDilZG0lxf |
| amadan.net/oficina/estilo.git v0.0.0-20260929070257-7a1a376ad2ab/go.mod h1:4u0jNBtIdMLOsJyrScfqoEuYDJF7BBM3qM9YWbzt6aw= |
| amadan.net/oficina/estilo.git v0.0.0-20260929070530-5b2ce835e773 h1:9DIAhG3TaGG+LE2+8LvU73ZEhfK+oNn2y2p4kFjAAi0= |
| amadan.net/oficina/estilo.git v0.0.0-20260929070530-5b2ce835e773/go.mod h1:4u0jNBtIdMLOsJyrScfqoEuYDJF7BBM3qM9YWbzt6aw= |
| +amadan.net/oficina/estilo.git v0.0.0-20260929082849-4d41ba2100ff h1:eGeR7E6hcL8b1z6FR1v0LkoY58lVyVLju7xvz39gJkQ= |
| +amadan.net/oficina/estilo.git v0.0.0-20260929082849-4d41ba2100ff/go.mod h1:4u0jNBtIdMLOsJyrScfqoEuYDJF7BBM3qM9YWbzt6aw= |
| amadan.net/oficina/home/handoff v0.0.0-20260908102137-481d2d57f489 h1:RPKtyry/71i06o8L+gjAaUyexbmG56kO1YPs601URAM= |
| amadan.net/oficina/home/handoff v0.0.0-20260908102137-481d2d57f489/go.mod h1:YISpzFJ+iMgrM0rUL032OSzsAQbP4/nozvXLCCOcn1k= |
| amadan.net/oficina/home/roster v0.0.0-20260908123429-74dda570e0cf h1:SRKjCUgtk42WwR7VSn+V0eLNk20mfiY8dQmzdw13Sbo= |
diff --git a/internal/home/assetversion.go b/internal/home/assetversion.go| index 06378c4..bcbc7cd 100644 |
| --- a/internal/home/assetversion.go |
| +++ b/internal/home/assetversion.go |
| @@ -61,13 +61,20 @@ func versionStaticRefs(page []byte) []byte { |
| }) |
| } |
| -// isVersionedAsset reports whether a path is one Home versions: used by |
| -// tests that check every page links assets by version. |
| -func isVersionedAsset(path string) bool { |
| - assetVersionsMu.RLock() |
| - defer assetVersionsMu.RUnlock() |
| - _, ok := assetVersions[strings.SplitN(path, "?", 2)[0]] |
| - return ok |
| +// etagMatches is If-None-Match's weak comparison (RFC 9110 §13.1.2): |
| +// any listed tag, W/ or not, naming the same opaque value matches, and so |
| +// does "*". Weak matters here: whatever gzips oficina.works on the way out |
| +// turns this binary's strong tag into W/"...", and an exact compare would |
| +// then never answer 304. |
| +func etagMatches(header, etag string) bool { |
| + want := strings.TrimPrefix(etag, "W/") |
| + for _, tag := range strings.Split(header, ",") { |
| + tag = strings.TrimSpace(tag) |
| + if tag == "*" || strings.TrimPrefix(tag, "W/") == want { |
| + return true |
| + } |
| + } |
| + return false |
| } |
| // mountCSS mounts a versioned stylesheet. |
diff --git a/internal/home/assetversion_test.go b/internal/home/assetversion_test.go| index c768988..1de50ef 100644 |
| --- a/internal/home/assetversion_test.go |
| +++ b/internal/home/assetversion_test.go |
| @@ -54,3 +54,21 @@ func TestAssetsAreLinkedByVersionAndCachedForAYear(t *testing.T) { |
| } |
| } |
| } |
| + |
| +func TestIfNoneMatchIsWeakAndListed(t *testing.T) { |
| + tag := `"0123456789abcdef"` |
| + for header, want := range map[string]bool{ |
| + tag: true, |
| + "W/" + tag: true, |
| + `"other", W/` + tag: true, |
| + ` "other" , ` + tag + ` `: true, |
| + "*": true, |
| + `"other"`: false, |
| + "": false, |
| + `W/"0123456789abcdeX"`: false, |
| + } { |
| + if got := etagMatches(header, tag); got != want { |
| + t.Errorf("etagMatches(%q) = %v, want %v", header, got, want) |
| + } |
| + } |
| +} |
diff --git a/internal/home/origins_test.go b/internal/home/origins_test.go| index abae455..a36335a 100644 |
| --- a/internal/home/origins_test.go |
| +++ b/internal/home/origins_test.go |
| @@ -31,7 +31,11 @@ func examplePath(route string) string { |
| case "/static/estilo/fonts/v1/*": |
| return theme.FontPrefix + "onest-latin.woff2" |
| case "/static/estilo/sky/v1/*": |
| - return sky.For(sky.SuiteSeed).Layers[0].Src |
| + for _, layer := range sky.For(sky.SuiteSeed).Layers { |
| + if layer.Src != "" { // the placeholder is inline, with no URL |
| + return layer.Src |
| + } |
| + } |
| } |
| return route |
| } |
diff --git a/internal/home/page.go b/internal/home/page.go| index 6bdedfa..e22ef60 100644 |
| --- a/internal/home/page.go |
| +++ b/internal/home/page.go |
| @@ -1,6 +1,9 @@ |
| package home |
| import ( |
| + "html/template" |
| + |
| + "amadan.net/oficina/estilo.git/sky" |
| "bytes" |
| "embed" |
| "net/http" |
| @@ -62,6 +65,20 @@ func mountSite(r chi.Router) { |
| mountCSS(r, "/static/site/apps.css", appsCSS) |
| } |
| +// skyMarker is where the front page's sky goes: rendered from estilo's |
| +// own template, so the page carries exactly the layers (and the inline |
| +// placeholder) the pinned estilo draws, never a hand-copied set of URLs. |
| +const skyMarker = "<!-- estilo-sky: the suite's sky, rendered from estilo's template on first serve (page.go) -->" |
| + |
| +func withSky(page []byte) []byte { |
| + t := template.Must(template.ParseFS(sky.Templates(), "templates/*.html")) |
| + var b bytes.Buffer |
| + if err := t.ExecuteTemplate(&b, "estilo-sky", sky.For(sky.SuiteSeed)); err != nil { |
| + panic(err) // estilo's own template over its own data; a broken pin |
| + } |
| + return bytes.Replace(page, []byte(skyMarker), b.Bytes(), 1) |
| +} |
| + |
| // servedIndex is the front page with every stylesheet and picture linked |
| // by its content version. Built on first serve, once the routers have |
| // mounted (and so versioned) everything the page links. |
| @@ -72,7 +89,7 @@ var ( |
| // handleIndex serves the front page. |
| func (a *App) handleIndex(w http.ResponseWriter, _ *http.Request) { |
| - servedIndexOnce.Do(func() { servedIndex = versionStaticRefs(indexPage) }) |
| + servedIndexOnce.Do(func() { servedIndex = versionStaticRefs(withSky(indexPage)) }) |
| w.Header().Set("Content-Type", "text/html; charset=utf-8") |
| w.Write(servedIndex) |
| } |
diff --git a/internal/home/page_test.go b/internal/home/page_test.go| index b79c7f2..b43de74 100644 |
| --- a/internal/home/page_test.go |
| +++ b/internal/home/page_test.go |
| @@ -2,6 +2,7 @@ package home |
| import ( |
| "bytes" |
| + "html" |
| "regexp" |
| "testing" |
| @@ -20,20 +21,29 @@ func TestFrontPageLineBreaksKeepTheirSpace(t *testing.T) { |
| } |
| } |
| -// The front page is static bytes, so it names estilo's versioned URLs |
| -// directly. When estilo moves one (a new sky drawing, a new font file), |
| -// this fails rather than the page quietly linking files that 404. |
| -func TestFrontPageLinksEstilosCurrentURLs(t *testing.T) { |
| - want := []string{theme.FontPrefix + "fonts.css"} |
| - for _, layer := range sky.For(sky.SuiteSeed).Layers { |
| - want = append(want, layer.Src) |
| +// The front page renders its sky from estilo's template: every layer the |
| +// pinned estilo draws, the inline placeholders included, and the marker |
| +// gone. |
| +func TestFrontPageRendersEstilosSky(t *testing.T) { |
| + if !bytes.Contains(indexPage, []byte(skyMarker)) { |
| + t.Fatal("index.html has no sky marker") |
| + } |
| + // Attribute values are HTML-escaped (html/template writes "+" as |
| + // "+"); the browser reads them unescaped, so compare that. |
| + page := []byte(html.UnescapeString(string(withSky(indexPage)))) |
| + if bytes.Contains(page, []byte(skyMarker)) { |
| + t.Fatal("the sky marker was not replaced") |
| } |
| - for _, url := range want { |
| - if !bytes.Contains(indexPage, []byte(`"`+url+`"`)) { |
| - t.Errorf("front page does not link %s", url) |
| + for _, layer := range sky.For(sky.SuiteSeed).Layers { |
| + want := layer.Src |
| + if want == "" { |
| + want = string(layer.Inline) |
| + } |
| + if !bytes.Contains(page, []byte(`src="`+want+`"`)) { |
| + t.Errorf("front page lacks the %s layer (%s)", layer.Name, layer.Tone) |
| } |
| } |
| - if n := bytes.Count(indexPage, []byte(sky.Prefix)); n != len(sky.For(sky.SuiteSeed).Layers) { |
| - t.Errorf("front page links %d sky files, want exactly the suite sky's %d", n, len(sky.For(sky.SuiteSeed).Layers)) |
| + if !bytes.Contains(indexPage, []byte(`"`+theme.FontPrefix+`fonts.css"`)) { |
| + t.Errorf("front page does not link %sfonts.css", theme.FontPrefix) |
| } |
| } |
diff --git a/internal/home/passkey.go b/internal/home/passkey.go| index d19fa1d..b351c1b 100644 |
| --- a/internal/home/passkey.go |
| +++ b/internal/home/passkey.go |
| @@ -213,7 +213,7 @@ func serveAsset(body []byte, contentType string) http.HandlerFunc { |
| } else { |
| w.Header().Set("Cache-Control", "no-cache") |
| } |
| - if r.Header.Get("If-None-Match") == etag { |
| + if etagMatches(r.Header.Get("If-None-Match"), etag) { |
| w.WriteHeader(http.StatusNotModified) |
| return |
| } |
diff --git a/internal/home/static/site/index.html b/internal/home/static/site/index.html| index 23e596b..2096157 100644 |
| --- a/internal/home/static/site/index.html |
| +++ b/internal/home/static/site/index.html |
| @@ -35,13 +35,7 @@ |
| <!-- ───────── Hero: every app's colour, together, behind the headline ───────── --> |
| <section class="hero" estilo-sky-host> |
| - <div estilo-sky aria-hidden="true"> |
| - <img estilo-sky-layer="haze" estilo-sky-tone="light" src="/static/estilo/sky/v1/fffadbd9c1aa2b94/haze-light.svg" alt="" loading="lazy" decoding="async"> |
| - <img estilo-sky-layer="haze" estilo-sky-tone="dark" src="/static/estilo/sky/v1/fffadbd9c1aa2b94/haze-dark.svg" alt="" loading="lazy" decoding="async"> |
| - <img estilo-sky-layer="clouds" estilo-sky-tone="light" src="/static/estilo/sky/v1/fffadbd9c1aa2b94/clouds-light.svg" alt="" loading="lazy" decoding="async"> |
| - <img estilo-sky-layer="clouds" estilo-sky-tone="dark" src="/static/estilo/sky/v1/fffadbd9c1aa2b94/clouds-dark.svg" alt="" loading="lazy" decoding="async"> |
| - <img estilo-sky-layer="filaments" src="/static/estilo/sky/v1/fffadbd9c1aa2b94/filaments.svg" alt="" loading="lazy" decoding="async"> |
| - </div> |
| + <!-- estilo-sky: the suite's sky, rendered from estilo's template on first serve (page.go) --> |
| <div class="shell hero-in"> |
| <h1 class="hero-h">Open your new office suite.</h1> |
| <p class="hero-lead">Oficina is fast, modern and light enough to forget it’s running. |