bdeshi/khosra
0
0
Fork 0
Code Issues Pull Requests Releases Activity

move robots and sitemap out of core, and raise the ceiling on purpose

Browse Source 
Item 0 of the roadmap's order of work, and it blocked everything after it: core
sat at 2965 of 3000 while the review scheduled four core-bound items, the first
of which — logging — wanted the whole remainder.

/robots.txt and /sitemap.xml are exact paths somebody else's software asks for by
name. They own no core concept and pass every test the architecture applies to a
feature; they lived in internal/web only because a feature could not own a route
until ADR-0081. internal/ext/discover/ now holds them. Core 2965 → 2913.

The seam gained one parameter to make it possible: a func() *content.Site, since a
sitemap must list what is served now and the index is swapped whole on every
rebuild (ADR-0077). A captured pointer would have frozen the site at startup —
which is the kind of bug that only shows up after a rebuild, in production.

The ceiling rises to 3400 as well as the move, because the move alone could not buy
the room. feed.go and web/extras.go cannot follow discover out: a feed lives at
/{section}/feed.xml and extras under a bundle's own URL, so both are resolver cases
while the seam mounts exact paths only. Raising by the minimum that unblocks one
item produces a ceiling nobody believes, so 3400 fits the View cluster with
headroom. HARNESS.md asks that a raise be read as evidence something belongs in
ext before evidence the number was small; both readings were true, so both actions
were taken.

web no longer reserves those two paths, so a clash between features is wire.go's:
it merges route maps in declaration order, keeps the earlier claim, logs the loser.
Verified — a site shipping root/robots.txt starts, serves the engine's robots.txt,
and logs the passthrough claim, where an unguarded mux.Handle would have panicked.

Evidence: robots.txt and sitemap.xml are byte-identical before and after the move
against the demo site (67 and 2701 bytes, cmp clean), and the sitemap keeps its
application/xml type.

One real cost, recorded in both places rather than hidden. internal/web's
visibility test asserted that a listing, a feed *and* a sitemap all hide
unpublished bundles — one property, one test, because all three share a Query. The
sitemap half moved to the feature instead of a web test importing ext, which would
invert the one-way layering the architecture gate enforces. That property is now
asserted twice, once per package owning a surface.

Three gates caught real mistakes on the way: the staged-tree check found a partial
stage where git rm had staged a deletion while the caller edits were unstaged, the
coupling gates demanded state.md and HARNESS.md, and the nesting advisory rejected
a closure that put the merge loop one level too deep — fixed by making it a plain
function rather than tolerated.

Extensions 6 → 7. Routing cases unmoved: exact paths are mux entries, never
resolver cases, which is what that counter's exclusion column already said.

13 files. Core 2913/3400, ext 2495/3500.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
bdeshi
2026-08-03 16:26:40 +06:00
co-authored by Claude Opus 5
parent b313b729d9
commit 69a7eb4733
14 changed files with 212 additions and 74 deletions
Download Patch File Download Diff File Expand all files Collapse all files
internal/web/discover.go
-71
View File
@@ -1,71 +0,0 @@
package web
import (
"fmt"
"io/fs"
"log/slog"
"net/http"
"strings"
"khosra/internal/content"
)
// robots and sitemap are the two files a crawler looks for by exact name.
const (
robotsPath = "/robots.txt"
sitemapPath = "/sitemap.xml"
)
// serveRobots answers /robots.txt, preferring the site's own file.
//
// A site that ships robots.txt has said something deliberate, so it is served verbatim; otherwise the engine
// emits the minimum that is true — everything is public, and here is the sitemap. The Sitemap line only
// appears with a declared base, because a relative sitemap reference is not something a crawler accepts.
func serveRobots(w http.ResponseWriter, req *http.Request, siteFS fs.FS, base string) {
if siteFS != nil {
if data, err := fs.ReadFile(siteFS, "robots.txt"); err == nil {
writeAs(w, "text/plain; charset=utf-8", data, "robots.txt")
return
}
}
var out strings.Builder
out.WriteString("User-agent: *\nDisallow:\n")
if base != "" {
fmt.Fprintf(&out, "Sitemap: %s\n", content.Absolute(base, sitemapPath))
}
writeAs(w, "text/plain; charset=utf-8", []byte(out.String()), "robots.txt")
}
// serveSitemap answers /sitemap.xml with every bundle in every language it exists in.
//
// It needs a declared base: the sitemap format has no room for a relative URL, so without one the honest
// answer is that this file does not exist rather than a file full of paths no crawler can use (ADR-0039).
// Every URL comes from content.URL, like every other path the engine emits, so a sitemap can never disagree
// with what is actually served.
func serveSitemap(w http.ResponseWriter, req *http.Request, site *content.Site, base string) {
if base == "" {
slog.Warn("no sitemap: the site declares no base URL", "file", content.SettingsFile)
http.NotFound(w, req)
return
}
var out strings.Builder
out.WriteString(`<?xml version="1.0" encoding="utf-8"?>` + "\n")
out.WriteString(`<urlset xmlns="http://www.sitemaps.org/schemas/sitemap/0.9">` + "\n")
for _, entry := range site.Everything() {
fmt.Fprintf(&out, "<url><loc>%s</loc>", xmlEscape(content.Absolute(base, content.URL(entry.Route, entry.Lang))))
if !entry.Date.IsZero() {
fmt.Fprintf(&out, "<lastmod>%s</lastmod>", entry.Date.Format("2006-01-02"))
}
out.WriteString("</url>\n")
}
out.WriteString("</urlset>\n")
writeAs(w, "application/xml; charset=utf-8", []byte(out.String()), "sitemap.xml")
}
// xmlEscape escapes the five characters XML reserves. A URL should contain none of them, and a sitemap that
// silently breaks on the one that does is worse than a slightly paranoid replacement.
func xmlEscape(s string) string {
return strings.NewReplacer(
"&", "&amp;", "<", "&lt;", ">", "&gt;", `"`, "&quot;", "'", "&apos;",
).Replace(s)
}
internal/web/discover_test.go
-101
View File
@@ -1,101 +0,0 @@
package web
import (
"net/http"
"net/http/httptest"
"strings"
"testing"
"testing/fstest"
"khosra/internal/content"
"khosra/internal/render"
)
func crawlerHandler(t *testing.T, settings content.Settings, extra fstest.MapFS) http.Handler {
t.Helper()
fsys := fstest.MapFS{
"content/posts/hello.md": {Data: []byte("---\ntitle: Hello\ndate: 2026-03-08\n---\nx\n")},
"content/posts/hello.bn.md": {Data: []byte("---\ntitle: হ্যালো\ndate: 2026-03-08\n---\nx\n")},
"content/pages/about.md": {Data: []byte("---\ntitle: About\n---\nx\n")},
}
for name, file := range extra {
fsys[name] = file
}
bundles, err := content.Scan(fsys)
if err != nil {
t.Fatal(err)
}
r, err := render.New(nil, settings, nil)
if err != nil {
t.Fatal(err)
}
return Handler(Fixed(content.NewSite(bundles), r), fsys, nil, settings, nil)
}
func TestSitemapListsEveryVariantAbsolutely(t *testing.T) {
h := crawlerHandler(t, content.Settings{Base: "https://khosra.example"}, nil)
rec := httptest.NewRecorder()
h.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/sitemap.xml", nil))
if rec.Code != http.StatusOK {
t.Fatalf("got %d, want 200", rec.Code)
}
if ct := rec.Header().Get("Content-Type"); !strings.HasPrefix(ct, "application/xml") {
t.Errorf("content-type = %q — a sitemap served as HTML is a sitemap nothing reads", ct)
}
body := rec.Body.String()
for _, want := range []string{
"<loc>https://khosra.example/posts/hello/</loc>",
"<loc>https://khosra.example/bn/posts/hello/</loc>", // each language is its own URL
"<loc>https://khosra.example/pages/about/</loc>",
"<lastmod>2026-03-08</lastmod>",
} {
if !strings.Contains(body, want) {
t.Errorf("missing %q:\n%s", want, body)
}
}
if strings.Contains(body, "<lastmod></lastmod>") {
t.Error("an undated bundle should carry no lastmod at all")
}
}
func TestNoBaseMeansNoSitemap(t *testing.T) {
// The format has no room for a relative URL, so the honest answer is that the file does not exist
// (ADR-0039) rather than one full of paths no crawler can resolve.
h := crawlerHandler(t, content.Settings{}, nil)
rec := httptest.NewRecorder()
h.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/sitemap.xml", nil))
if rec.Code != http.StatusNotFound {
t.Errorf("got %d, want 404", rec.Code)
}
}
func TestRobotsIsGeneratedOrTheSitesOwn(t *testing.T) {
h := crawlerHandler(t, content.Settings{Base: "https://khosra.example"}, nil)
rec := httptest.NewRecorder()
h.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/robots.txt", nil))
body := rec.Body.String()
if !strings.Contains(body, "User-agent: *") || !strings.Contains(body, "Sitemap: https://khosra.example/sitemap.xml") {
t.Errorf("generated robots should point at the sitemap:\n%s", body)
}
if ct := rec.Header().Get("Content-Type"); !strings.HasPrefix(ct, "text/plain") {
t.Errorf("content-type = %q", ct)
}
// A site that ships its own has said something deliberate.
h = crawlerHandler(t, content.Settings{Base: "https://khosra.example"},
fstest.MapFS{"robots.txt": {Data: []byte("User-agent: *\nDisallow: /drafts/\n")}})
rec = httptest.NewRecorder()
h.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/robots.txt", nil))
if got := rec.Body.String(); !strings.Contains(got, "Disallow: /drafts/") || strings.Contains(got, "Sitemap:") {
t.Errorf("the site's own robots.txt should be served verbatim:\n%s", got)
}
}
func TestRobotsWithoutABaseOmitsTheSitemapLine(t *testing.T) {
h := crawlerHandler(t, content.Settings{}, nil)
rec := httptest.NewRecorder()
h.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/robots.txt", nil))
if got := rec.Body.String(); strings.Contains(got, "Sitemap:") {
t.Errorf("a relative sitemap reference is not something a crawler accepts:\n%s", got)
}
}
internal/web/slug_test.go
+3 -6
View File
@@ -68,7 +68,9 @@ func TestAnAliasCanKeepTheOldPathWorking(t *testing.T) {
}
}
func TestListingsAndSitemapsUseTheSluggedAddress(t *testing.T) {
// The sitemap half of this moved to internal/ext/discover with the sitemap itself (ADR-0085); a web test
// cannot reach a feature, since the layering runs one way only (conventions.md).
func TestListingsUseTheSluggedAddress(t *testing.T) {
fsys := fstest.MapFS{
"content/posts/hello-world.md": {Data: []byte("---\ntitle: Hello\ndate: 2026-03-01\nslug: ekti-post\n---\nx\n")},
"content/posts/plain.md": {Data: []byte("---\ntitle: Plain\ndate: 2026-02-01\n---\nx\n")},
@@ -89,11 +91,6 @@ func TestListingsAndSitemapsUseTheSluggedAddress(t *testing.T) {
if body := rec.Body.String(); !strings.Contains(body, `href="/posts/ekti-post/"`) || strings.Contains(body, "hello-world") {
t.Errorf("a listing must link the address, not the key:\n%s", body)
}
rec = httptest.NewRecorder()
h.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/sitemap.xml", nil))
if body := rec.Body.String(); !strings.Contains(body, "/posts/ekti-post/") || strings.Contains(body, "hello-world") {
t.Errorf("a sitemap that disagrees with what is served is worse than none:\n%s", body)
}
}
func TestAmbiguousOrCollidingSlugsAreDropped(t *testing.T) {
internal/web/visibility_test.go
+7 -2
View File
@@ -69,8 +69,13 @@ func TestNothingInsideAnUnpublishedBundleIsServed(t *testing.T) {
}
func TestUnpublishedBundlesAreAbsentFromEverythingThatLists(t *testing.T) {
// A listing, a feed and a sitemap all go through the same Query, so hiding a draft in one place hides it
// A listing and a feed both go through the same Query, so hiding a draft in one place hides it
// everywhere. That is the property worth testing rather than each surface separately.
//
// The sitemap was the third surface here until ADR-0085 moved it to internal/ext/discover. Its half of
// this test moved with it rather than reaching across the boundary: a web test importing a feature would
// invert the one-way layering the architecture gate enforces. The property is now asserted twice, once
// per package that owns a surface — a real cost of the move, recorded rather than hidden.
fsys := unpublishedFS()
fsys["content/art/live/index.md"] = &fstest.MapFile{Data: []byte("---\ntitle: Live\ndate: 2020-01-01\n---\nx\n")}
bundles, err := content.Scan(fsys)
@@ -83,7 +88,7 @@ func TestUnpublishedBundlesAreAbsentFromEverythingThatLists(t *testing.T) {
t.Fatal(err)
}
h := Handler(Fixed(content.NewSite(bundles), r), fsys, nil, settings, nil)
for _, path := range []string{"/art/", "/feed.xml", "/sitemap.xml"} {
for _, path := range []string{"/art/", "/feed.xml"} {
rec := httptest.NewRecorder()
h.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, path, nil))
body := rec.Body.String()
internal/web/web.go
+7 -14
View File
@@ -42,21 +42,13 @@ func Fixed(site *content.Site, theme *render.Renderer) Current {
// contain, is the feature's business — the same division `/derived/` already uses.
func Handler(current Current, siteFS, derivedFS fs.FS, settings content.Settings, routes map[string]http.Handler) http.Handler {
mux := http.NewServeMux()
// Registered first so a later duplicate is caught rather than panicking, and so core's own answers are
// the ones that cannot be taken over.
reserved := map[string]bool{"/": true, robotsPath: true, sitemapPath: true}
// Core's own answers, which a feature may not take over. /robots.txt and /sitemap.xml are no longer here:
// they moved to a feature (ADR-0085), so a clash between two features is wire.go's to detect.
reserved := map[string]bool{"/": true}
mux.HandleFunc("GET /", func(w http.ResponseWriter, req *http.Request) {
now := current()
serve(w, req, now.Site, now.Theme, siteFS, settings)
})
// Two exact paths a crawler asks for by name, so they are mux entries rather than resolver cases: no
// bundle can own them, since a key always sits under a section.
mux.HandleFunc("GET "+robotsPath, func(w http.ResponseWriter, req *http.Request) {
serveRobots(w, req, siteFS, settings.Base)
})
mux.HandleFunc("GET "+sitemapPath, func(w http.ResponseWriter, req *http.Request) {
serveSitemap(w, req, current().Site, settings.Base)
})
if siteFS != nil {
if sub, err := fs.Sub(siteFS, "static"); err == nil {
mux.Handle("GET /static/", http.StripPrefix("/static/", serveStatic(sub)))
@@ -69,11 +61,12 @@ func Handler(current Current, siteFS, derivedFS fs.FS, settings content.Settings
http.StripPrefix(content.DerivedPrefix, serveStatic(derivedFS)))
}
// A feature's routes go on last. A path core already answers is skipped, not overridden: http.ServeMux
// panics on a duplicate pattern, so without this a site shipping root/robots.txt would take the server
// down at startup rather than lose a race it was never told about. `khosra check` reports the shadow.
// panics on a duplicate pattern, so without this a feature claiming "/" would take the server down at
// startup rather than lose a race it was never told about. Clashes *between* features are settled in
// cmd/khosra/wire.go, which is the only place that knows features exist.
for pattern, handler := range routes {
if reserved[pattern] || strings.HasPrefix(pattern, "/static/") || strings.HasPrefix(pattern, content.DerivedPrefix) {
slog.Warn("a passthrough path is already answered by the engine and is not served",
slog.Warn("a feature claims a path the engine already answers; it is not served",
"path", pattern)
continue
}