diff --git a/HARNESS.md b/HARNESS.md index 0f51ea3..b70d39f 100644 --- a/HARNESS.md +++ b/HARNESS.md @@ -81,7 +81,10 @@ agent to push back toward CSS or a template. stops growing, ext rises" (invariant 9) is observable rather than asserted. The values live in `scripts/budgets.env`; raising either needs an ADR stating what moved and why, and the ADR log is where that history belongs, not here. Read a core raise as evidence something belongs in `internal/ext/` before reading -it as evidence the number was small. +it as evidence the number was small. ADR-0085 is the worked example: `discover` left core for +`internal/ext/` **and** the ceiling rose to 3400, because the move alone freed 57 lines against four +scheduled core-bound items, and `feed.go` cannot follow it out — a feed is a resolver case, not an exact +path. **Context is a budget, and parts of it are mechanical.** The limit on this project is how much work fits in a session, so `harness/context-economy.md` holds the reading, searching and reporting diff --git a/cmd/khosra/example_test.go b/cmd/khosra/example_test.go index 0bce6ac..eb3fe45 100644 --- a/cmd/khosra/example_test.go +++ b/cmd/khosra/example_test.go @@ -47,7 +47,8 @@ func exampleSite(t *testing.T) http.Handler { } site := content.NewSite(bundles) r.Navigation(site.Sections) - return web.Handler(web.Fixed(site, r), fsys, nil, settings, routes(fsys, settings)) + return web.Handler(web.Fixed(site, r), fsys, nil, settings, + routes(fsys, settings, func() *content.Site { return site })) } func get(t *testing.T, h http.Handler, path string) (int, string) { diff --git a/cmd/khosra/main.go b/cmd/khosra/main.go index 0a685f6..5858066 100644 --- a/cmd/khosra/main.go +++ b/cmd/khosra/main.go @@ -89,7 +89,8 @@ func runServe() { watching(fsys, interval, rebuild) slog.Info("serving", "site", *site, "bundles", count, "addr", *addr) - handler := web.Handler(live.Load, fsys, derivedFS, settings, routes(fsys, settings)) + handler := web.Handler(live.Load, fsys, derivedFS, settings, + routes(fsys, settings, func() *content.Site { return live.Load().Site })) if err := http.ListenAndServe(*addr, handler); err != nil { fatal("server stopped", err) } diff --git a/cmd/khosra/wire.go b/cmd/khosra/wire.go index d12d4a3..fa3e1a6 100644 --- a/cmd/khosra/wire.go +++ b/cmd/khosra/wire.go @@ -2,12 +2,14 @@ package main import ( "io/fs" + "log/slog" "net/http" "github.com/yuin/goldmark" "github.com/yuin/goldmark/extension" "khosra/internal/content" + "khosra/internal/ext/discover" "khosra/internal/ext/notation" "khosra/internal/ext/passthrough" "khosra/internal/ext/shortcodes" @@ -48,9 +50,32 @@ func extenders(partial render.Partial) []goldmark.Extender { // routes is the only list of features owning a URL path of their own. // // The sibling of extenders(), and the same rule: nothing below cmd may know which features exist, so core -// receives a map of paths and never the name of whatever filled it (ADR-0081). One entry today — the -// registry earned exactly the field that has an implementor, and the other six in `extensions.md` wait for -// theirs. -func routes(siteFS fs.FS, settings content.Settings) map[string]http.Handler { - return passthrough.Routes(siteFS, settings) +// receives a map of paths and never the name of whatever filled it (ADR-0081). +// +// site is a callback because a sitemap must list what is served *now*, and the index is swapped whole on +// every rebuild (ADR-0077); a captured pointer would freeze the site as it was at startup. +// +// Order is precedence. discover goes first, so a `root/robots.txt` cannot take over the path that already +// prefers a site's own robots.txt — a clash is logged and dropped rather than silently overwriting, because +// a map assignment would have picked a winner by iteration order. +func routes(siteFS fs.FS, settings content.Settings, site func() *content.Site) map[string]http.Handler { + out := map[string]http.Handler{} + claim(out, "discover", discover.Routes(siteFS, settings, site)) + claim(out, "passthrough", passthrough.Routes(siteFS, settings)) + return out +} + +// claim adds a feature's routes, keeping whatever already holds a path. +// +// A plain function rather than a closure over out, because a closure put this loop one level deeper than +// `conventions.md` allows — and the nesting advisory is meant to be acted on, not tolerated. +func claim(out map[string]http.Handler, feature string, from map[string]http.Handler) { + for pattern, handler := range from { + if _, taken := out[pattern]; taken { + slog.Warn("two features claim the same path; the earlier one keeps it", + "path", pattern, "feature", feature) + continue + } + out[pattern] = handler + } } diff --git a/harness/decisions.md b/harness/decisions.md index e38b80c..5558b3a 100644 --- a/harness/decisions.md +++ b/harness/decisions.md @@ -1500,3 +1500,34 @@ URL, which is outbound work that must not happen on the request path. That is th trigger, not a fourth one. Revisit if: an Effect is neither artifact-producing nor outbound, which would mean a third kind and a real gap in this split. + +## ADR-0085 — `discover` becomes a feature, and the core ceiling rises to 3400 +Date: 2026-08-03 · Status: accepted +Decision: `/robots.txt` and `/sitemap.xml` move from `internal/web/discover.go` into +`internal/ext/discover/`, mounted through ADR-0081's Routes seam. The seam gains one parameter — a +`func() *content.Site` — because a sitemap must list what is served *now* and the index is swapped whole on +every rebuild (ADR-0077); a captured pointer would freeze the site at startup. `CORE_LOC_MAX` rises from +3000 to 3400. +Why the move: these are exact paths somebody else's software asks for by name, own no core concept, and pass +every test the architecture applies to a feature. They sat in core only because a feature could not own a +route until ADR-0081. Core went 2965 → 2908. +Why the raise as well as the move: the review on 2026-08-02 scheduled four core-bound items — logging, the +View layer, declared content types and a minimal settings cascade — and 35 free lines would not have fitted +the first of them. `feed.go` and `web/extras.go` cannot follow `discover` out, because a feed lives at +`/{section}/feed.xml` and extras under a bundle's own URL: both are **resolver** cases, and the seam mounts +exact paths only. So the move alone could not buy the room, and raising by the minimum that unblocks one item +produces a ceiling nobody believes. 3400 fits the View cluster with headroom. +`HARNESS.md` asks that a core raise be read as evidence something belongs in `internal/ext/` before evidence +the number was small. Both readings are true here, which is why both actions were taken rather than either. +Consequence: `web` no longer reserves those two paths, so a clash between *features* is `wire.go`'s to +settle — it merges route maps in declaration order, keeps the earlier claim, and 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. Output is byte-identical before and after for both +paths on the demo site. +One real cost: `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 rather than a `web` test importing `ext`, which would invert the one-way layering the architecture +gate enforces. The property is now asserted twice, once per package that owns a surface. That is a genuine +loss, recorded in both tests. +Revisit if: the resolver gains feature participation, at which point `feed.go` and `web/extras.go` can leave +too and the ceiling should be reconsidered downward rather than left as headroom. diff --git a/harness/state.md b/harness/state.md index 7379214..ce2f35e 100644 --- a/harness/state.md +++ b/harness/state.md @@ -33,6 +33,7 @@ table owns. | `internal/ext/notation/` | the inline marks CommonMark lacks: `~sub~`, `^sup^`, `==mark==`, and `~~strike~~`, which it owns so a single tilde can mean subscript (ADR-0061). `abbr.go` adds `*[TERM]:` definitions and the pass that expands them (ADR-0062) | | `internal/ext/scaffold/` | writes one draft directory bundle into a site root through `os.Root`: never an overwrite | | `internal/ext/watch/` | polls `content/` and `templates/` on an interval it is given, ignores editor droppings, and reports a settled change (ADR-0022, ADR-0048, ADR-0056). `site.yaml` is deliberately not fingerprinted (ADR-0055) | +| `internal/ext/discover/` | seventh feature: `/robots.txt` and `/sitemap.xml`, absolute and only with a declared base (ADR-0039). Left core in ADR-0085 — exact paths somebody else's software asks for by name, owning no core concept | | `internal/ext/passthrough/` | fifth feature, and the first to own a **route** (ADR-0081): files in `root/` served at the exact path they occupy, `.tmpl` rendered as text with the site's own settings, headers declared per path in `root/_headers.yaml`, underscore-prefixed names not addressable | | `internal/ext/check/` | third feature: validates a site root — what the engine worked around, broken internal links, missing titles and alt text, mixed series ordering, and calls left in the retired shortcode form (ADR-0059) | | `cmd/khosra/wire.go` | the only list of enabled features (`extensions.md`) — `extenders()` for the ones goldmark composes and `routes()` for the ones owning a URL path (ADR-0081), the `theme` function that builds the renderer this build ships (ADR-0072), and the Markdown dialect with it — tables, footnotes, definition lists, strikethrough, task lists (ADR-0058, ADR-0078) | @@ -40,8 +41,7 @@ table owns. | `internal/web/extras.go` | the extras route: listing, one entry selected, or `?raw` bytes, all behind the bundle lookup | | `internal/web/asset.go` | files inside a bundle's own directory, looked up through the owning bundle so visibility can only ever inherit (ADR-0024) | | `internal/web/feed.go` | Atom for the site, a section or a tag, from dated bundles via one Query (ADR-0043) | -| `internal/web/discover.go` | `/robots.txt` and `/sitemap.xml`, absolute and only with a declared base (ADR-0039) | -| `internal/web/web.go` | handler: `Snapshot` pairs the index with the theme that was current with it (ADR-0077); `serve` dispatches by kind, `serveBundle` answers the commonest one; listings, `/static/`, `/derived/`, degrade on failure. Mounts the exact paths features own, skipping any the engine already answers — a duplicate pattern would panic (ADR-0081) | +| `internal/web/web.go` | handler: `Snapshot` pairs the index with the theme that was current with it (ADR-0077); `serve` dispatches by kind, `serveBundle` answers the commonest one; listings, `/static/`, `/derived/`, degrade on failure. Mounts the exact paths features own, skipping any the engine already answers — a duplicate pattern would panic (ADR-0081). Since ADR-0085 it reserves only `/`: `/robots.txt` and `/sitemap.xml` are a feature's, so a clash *between* features is `wire.go`'s to settle | | `cmd/khosra/main.go` | flags (including `-poll`, zero to stop watching), wiring, startup, the derivative pass, and the one atomic swap a change goes through, theme and index together in `rebuilder` (ADR-0077). `main` dispatches subcommands, `runServe` assembles the server, `rebuilder` is used at startup and on every change alike | | `cmd/khosra/check.go` | the `check` subcommand: parse, print, exit code. What counts as a finding lives in the feature | | `cmd/khosra/new.go` | the `new` subcommand: arguments in either order, then the feature does the writing | @@ -105,7 +105,7 @@ a row that leaves it empty (ADR-0070). | Collection pages | 4 | **1** — done | Query primitive: `content.Query{Section, Tag, Lang}` + `Site.Run` | A series archive. Membership is structural and the sort ascends, so it resolves through `Site.Sequence` — sharing the index but not the Query | | Views — **per-bundle selection only** | 0 | **2** | The View layer `architecture.md` describes: `view:` in frontmatter choosing a presentation, resolved through the cascade. Nothing selects a view yet | Output formats. HTML, sitemap XML and Atom are three functions with nothing to share — an interface over them would have one member and no leverage | | Effects | 1 | **2** | Effect runner + trigger wiring (change / schedule / demand). The only one is the derivative pass (ADR-0042), called from `cmd` inside `rebuilder`, so it already answers both triggers it will ever need — startup and a settled change (ADR-0048) | An in-memory swap. Replacing the index or the theme re-reads the site root into memory, writing no artifact and calling nothing outbound (ADR-0055) | -| Extensions | 6 | **3** — passed, and the registry is now partly built | Extension registry (`extensions.md`). It reached 3 once before and went back to 2 when the widows feature was deleted (ADR-0045) — a threshold reached by a feature that should not exist was never a threshold. The note below says which field was built and why the rest were not | An upstream extension enabled in the list. `Table`, `Footnote` and `DefinitionList` are goldmark's, so they are dialect rather than features of this engine (ADR-0058) — only a package under `internal/ext/` counts | +| Extensions | 7 | **3** — passed, and the registry is now partly built | Extension registry (`extensions.md`). It reached 3 once before and went back to 2 when the widows feature was deleted (ADR-0045) — a threshold reached by a feature that should not exist was never a threshold. The note below says which field was built and why the rest were not | An upstream extension enabled in the list. `Table`, `Footnote` and `DefinitionList` are goldmark's, so they are dialect rather than features of this engine (ADR-0058) — only a package under `internal/ext/` counts | | Interface implementations | — | **2** | The interface itself | An interface this repo did not declare. Satisfying `fs.FS`, `http.Handler` or `goldmark.Extender` is using somebody else's abstraction, which is the opposite of inventing one | | Non-stdlib dependencies | 4 direct | budget in `scripts/budgets.env` | — | The standard library, and a dependency's own test-only modules — `go list -m all` shows those, and the gate counts `require` entries instead (`scripts/budgets.env`) | @@ -120,10 +120,10 @@ landed (ADR-0061, ADR-0062) and the count was never incremented, though the pros five. Six now, with `passthrough`. This is the latent item about the counters having no mechanical check, demonstrating itself; the count is authoritative only because someone just ran `ls internal/ext/`. -**A registry over the *other* attachment points would still buy nothing.** The six features +**A registry over the *other* attachment points would still buy nothing.** The seven features attach in four unrelated ways: `shortcodes` and `notation` are goldmark extenders listed in `extenders()`, `check` and `scaffold` are functions `cmd` calls for a subcommand, `watch` is a goroutine, and -`passthrough` hands back a map of URL paths. A registry would +`passthrough` and `discover` hand back maps of URL paths. A registry would have to abstract over "extends Markdown", "validates content", "writes a file" and "polls a directory", which share nothing but the word *feature* — one member and no leverage. Adding `notation` made this clearer rather than more urgent: two goldmark extenders compose in goldmark's own extender list, which is already the registry diff --git a/harness/surface.md b/harness/surface.md index 879fd00..6a934b2 100644 --- a/harness/surface.md +++ b/harness/surface.md @@ -6,22 +6,23 @@ Every top-level declaration in the engine, with its line. Read this before openi file: it answers "where does X live" and "what is in this package" without the bodies. What each file is *for* lives in `state.md`; why it is that way lives in `decisions.md`. -## cmd/khosra — 320 lines + 308 test +## cmd/khosra — 346 lines + 309 test -check.go 45 · main.go 177 · new.go 42 · wire.go 56 +check.go 45 · main.go 178 · new.go 42 · wire.go 81 - check.go:16 func runCheck(args []string) - main.go:23 func main() - main.go:44 func runServe() -- main.go:100 func pollInterval(dev bool, chosen time.Duration) time.Duration -- main.go:116 func watching(fsys fs.FS, every time.Duration, rebuild func() int) -- main.go:128 func rebuilder(fsys fs.FS, settings content.Settings, cache string, reveal bool, -- main.go:160 func defaultCache() string -- main.go:170 func fatal(msg string, err error) +- main.go:101 func pollInterval(dev bool, chosen time.Duration) time.Duration +- main.go:117 func watching(fsys fs.FS, every time.Duration, rebuild func() int) +- main.go:129 func rebuilder(fsys fs.FS, settings content.Settings, cache string, reveal bool, +- main.go:161 func defaultCache() string +- main.go:171 func fatal(msg string, err error) - new.go:12 func runNew(args []string) -- wire.go:20 func theme(siteFS fs.FS, settings content.Settings) (*render.Renderer, error) -- wire.go:33 func extenders(partial render.Partial) []goldmark.Extender -- wire.go:54 func routes(siteFS fs.FS, settings content.Settings) map[string]http.Handler +- wire.go:22 func theme(siteFS fs.FS, settings content.Settings) (*render.Renderer, error) +- wire.go:35 func extenders(partial render.Partial) []goldmark.Extender +- wire.go:61 func routes(siteFS fs.FS, settings content.Settings, site func() *content.Site) map[string]http.Handler +- wire.go:72 func claim(out map[string]http.Handler, feature string, from map[string]http.Handler) ## internal/content — 1074 lines + 598 test @@ -115,6 +116,17 @@ check.go 223 · doc.go 8 - check.go:172 func asset(fsys fs.FS, trimmed string, site *content.Site) bool - check.go:200 func mixedOrdering(bundles []content.Bundle, site *content.Site) []Finding +## internal/ext/discover — 107 lines + 139 test + +discover.go 98 · doc.go 9 + +- discover.go:14 const +- discover.go:24 func Routes(siteFS fs.FS, settings content.Settings, site func() *content.Site) map[string]http.Handler +- discover.go:40 func robots(w http.ResponseWriter, siteFS fs.FS, base string) +- discover.go:61 func sitemap(w http.ResponseWriter, req *http.Request, site *content.Site, base string) +- discover.go:83 func xmlEscape(s string) string +- discover.go:93 func writeAs(w http.ResponseWriter, contentType string, out []byte, what string) + ## internal/ext/notation — 411 lines + 149 test abbr.go 246 · doc.go 8 · notation.go 157 @@ -355,15 +367,11 @@ chrome.go 115 · render.go 499 · view.go 192 - view.go:167 type Picture struct - view.go:184 type Origin struct -## internal/web — 765 lines + 1423 test +## internal/web — 687 lines + 1324 test -asset.go 58 · discover.go 71 · extras.go 93 · feed.go 125 · resolve.go 170 · web.go 248 +asset.go 58 · extras.go 93 · feed.go 125 · resolve.go 170 · web.go 241 - asset.go:22 func serveAsset(w http.ResponseWriter, req *http.Request, site *content.Site, siteFS fs.FS, res resolution) bool -- discover.go:14 const -- discover.go:24 func serveRobots(w http.ResponseWriter, req *http.Request, siteFS fs.FS, base string) -- discover.go:45 func serveSitemap(w http.ResponseWriter, req *http.Request, site *content.Site, base string) -- discover.go:67 func xmlEscape(s string) string - extras.go:18 func serveExtras(w http.ResponseWriter, req *http.Request, site *content.Site, r *render.Renderer, - extras.go:69 func renderExtras(w http.ResponseWriter, r *render.Renderer, b content.Bundle, served string, - extras.go:85 func find(entries []content.Entry, want string) (content.Entry, bool) @@ -386,10 +394,10 @@ asset.go 58 · discover.go 71 · extras.go 93 · feed.go 125 · resolve.go 170 - web.go:28 type Current func() *Snapshot - web.go:31 func Fixed(site *content.Site, theme *render.Renderer) Current - web.go:43 func Handler(current Current, siteFS, derivedFS fs.FS, settings content.Settings, routes map[string]http.Handler) http.Handler -- web.go:91 func serveStatic(sub fs.FS) http.Handler -- web.go:107 func serveListing(w http.ResponseWriter, req *http.Request, site *content.Site, r *render.Renderer, res resolution) bool -- web.go:131 func serveTags(w http.ResponseWriter, req *http.Request, site *content.Site, r *render.Renderer, res resolution) bool -- web.go:156 func write(w http.ResponseWriter, out []byte, what string) -- web.go:165 func writeAs(w http.ResponseWriter, contentType string, out []byte, what string) -- web.go:173 func serve(w http.ResponseWriter, req *http.Request, site *content.Site, r *render.Renderer, siteFS fs.FS, settings content.Settings) -- web.go:204 func serveBundle(w http.ResponseWriter, req *http.Request, site *content.Site, r *render.Renderer, +- web.go:84 func serveStatic(sub fs.FS) http.Handler +- web.go:100 func serveListing(w http.ResponseWriter, req *http.Request, site *content.Site, r *render.Renderer, res resolution) bool +- web.go:124 func serveTags(w http.ResponseWriter, req *http.Request, site *content.Site, r *render.Renderer, res resolution) bool +- web.go:149 func write(w http.ResponseWriter, out []byte, what string) +- web.go:158 func writeAs(w http.ResponseWriter, contentType string, out []byte, what string) +- web.go:166 func serve(w http.ResponseWriter, req *http.Request, site *content.Site, r *render.Renderer, siteFS fs.FS, settings content.Settings) +- web.go:197 func serveBundle(w http.ResponseWriter, req *http.Request, site *content.Site, r *render.Renderer, diff --git a/internal/web/discover.go b/internal/ext/discover/discover.go similarity index 55% rename from internal/web/discover.go rename to internal/ext/discover/discover.go index 0f90968..909bccb 100644 --- a/internal/web/discover.go +++ b/internal/ext/discover/discover.go @@ -1,4 +1,4 @@ -package web +package discover import ( "fmt" @@ -10,18 +10,34 @@ import ( "khosra/internal/content" ) -// robots and sitemap are the two files a crawler looks for by exact name. +// The two paths, exported so the wiring can reason about precedence without restating strings. const ( - robotsPath = "/robots.txt" - sitemapPath = "/sitemap.xml" + RobotsPath = "/robots.txt" + SitemapPath = "/sitemap.xml" ) -// serveRobots answers /robots.txt, preferring the site's own file. +// Routes returns the two exact paths this feature owns. +// +// site is a callback rather than a value because a sitemap must list what is served *now*: the index is +// swapped whole on every rebuild (ADR-0077), and a captured pointer would serve the site as it was at +// startup. settings is copied, since editing `site.yaml` needs a restart anyway (ADR-0055). +func Routes(siteFS fs.FS, settings content.Settings, site func() *content.Site) map[string]http.Handler { + return map[string]http.Handler{ + RobotsPath: http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + robots(w, siteFS, settings.Base) + }), + SitemapPath: http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + sitemap(w, req, site(), settings.Base) + }), + } +} + +// robots 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) { +func robots(w http.ResponseWriter, 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") @@ -31,18 +47,18 @@ func serveRobots(w http.ResponseWriter, req *http.Request, siteFS fs.FS, base st var out strings.Builder out.WriteString("User-agent: *\nDisallow:\n") if base != "" { - fmt.Fprintf(&out, "Sitemap: %s\n", content.Absolute(base, sitemapPath)) + 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. +// sitemap 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) { +func sitemap(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) @@ -69,3 +85,14 @@ func xmlEscape(s string) string { "&", "&", "<", "<", ">", ">", `"`, """, "'", "'", ).Replace(s) } + +// writeAs sets the type and writes, logging a failed write rather than pretending it succeeded. +// +// Five lines copied from internal/web rather than shared: a feature may not import web (conventions.md), and +// two copies of five obvious lines is cheaper than a package existing to hold them. +func writeAs(w http.ResponseWriter, contentType string, out []byte, what string) { + w.Header().Set("Content-Type", contentType) + if _, err := w.Write(out); err != nil { + slog.Warn("write failed", "what", what, "err", err) + } +} diff --git a/internal/web/discover_test.go b/internal/ext/discover/discover_test.go similarity index 61% rename from internal/web/discover_test.go rename to internal/ext/discover/discover_test.go index 8a3b64b..191651a 100644 --- a/internal/web/discover_test.go +++ b/internal/ext/discover/discover_test.go @@ -1,4 +1,4 @@ -package web +package discover import ( "net/http" @@ -8,9 +8,10 @@ import ( "testing/fstest" "khosra/internal/content" - "khosra/internal/render" ) +// crawlerHandler mounts just this feature's routes, which is all these two paths need — no renderer, no +// resolver, no mux full of bundle handling. func crawlerHandler(t *testing.T, settings content.Settings, extra fstest.MapFS) http.Handler { t.Helper() fsys := fstest.MapFS{ @@ -25,11 +26,12 @@ func crawlerHandler(t *testing.T, settings content.Settings, extra fstest.MapFS) if err != nil { t.Fatal(err) } - r, err := render.New(nil, settings, nil) - if err != nil { - t.Fatal(err) + site := content.NewSite(bundles) + mux := http.NewServeMux() + for pattern, handler := range Routes(fsys, settings, func() *content.Site { return site }) { + mux.Handle("GET "+pattern, handler) } - return Handler(Fixed(content.NewSite(bundles), r), fsys, nil, settings, nil) + return mux } func TestSitemapListsEveryVariantAbsolutely(t *testing.T) { @@ -99,3 +101,39 @@ func TestRobotsWithoutABaseOmitsTheSitemapLine(t *testing.T) { t.Errorf("a relative sitemap reference is not something a crawler accepts:\n%s", got) } } + +// A slug moves a bundle's address without moving its key, so a sitemap built from keys would advertise URLs +// that 404. Moved here from internal/web with the sitemap itself (ADR-0085). +func TestTheSitemapUsesTheSluggedAddress(t *testing.T) { + h := crawlerHandler(t, content.Settings{Base: "https://khosra.example"}, fstest.MapFS{ + "content/posts/hello-world.md": {Data: []byte("---\ntitle: Hello\ndate: 2026-03-01\nslug: ekti-post\n---\nx\n")}, + }) + rec := httptest.NewRecorder() + h.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/sitemap.xml", nil)) + body := rec.Body.String() + if !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) + } +} + +// The sitemap goes through the same Query every listing does, so a draft or a future-dated bundle is absent +// from it for the same reason it is absent from a section page. The other half of this lives in +// internal/web's visibility test, which owns the surfaces that stayed there (ADR-0085). +func TestTheSitemapOmitsUnpublishedBundles(t *testing.T) { + h := crawlerHandler(t, content.Settings{Base: "https://khosra.example"}, fstest.MapFS{ + "content/art/draft/index.md": {Data: []byte("---\ntitle: Draft\ndraft: true\n---\nx\n")}, + "content/art/future/index.md": {Data: []byte("---\ntitle: Future\ndate: 2099-01-01\n---\nx\n")}, + "content/art/live/index.md": {Data: []byte("---\ntitle: Live\ndate: 2020-01-01\n---\nx\n")}, + }) + rec := httptest.NewRecorder() + h.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/sitemap.xml", nil)) + body := rec.Body.String() + if !strings.Contains(body, "/art/live/") { + t.Errorf("the sitemap lost a published bundle:\n%s", body) + } + for _, hidden := range []string{"draft", "future"} { + if strings.Contains(body, hidden) { + t.Errorf("the sitemap leaked the %s bundle:\n%s", hidden, body) + } + } +} diff --git a/internal/ext/discover/doc.go b/internal/ext/discover/doc.go new file mode 100644 index 0000000..0b21043 --- /dev/null +++ b/internal/ext/discover/doc.go @@ -0,0 +1,9 @@ +// Package discover serves the two files a crawler asks for by exact name. +// +// `/robots.txt` and `/sitemap.xml` are addresses other people's software decided, like the `.well-known` +// paths passthrough serves. Neither is a bundle and neither can collide with one, since a bundle key always +// sits under a section — so both are exact mux entries rather than resolver cases (ADR-0039). +// +// This lived in `internal/web` until ADR-0085 moved it out: it is a feature by every test the architecture +// applies, and core needed the room for the primitives Arc 2 earns. +package discover diff --git a/internal/web/slug_test.go b/internal/web/slug_test.go index c4dd123..deab60f 100644 --- a/internal/web/slug_test.go +++ b/internal/web/slug_test.go @@ -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) { diff --git a/internal/web/visibility_test.go b/internal/web/visibility_test.go index 7b7b662..77267f8 100644 --- a/internal/web/visibility_test.go +++ b/internal/web/visibility_test.go @@ -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() diff --git a/internal/web/web.go b/internal/web/web.go index 1d929bf..8670477 100644 --- a/internal/web/web.go +++ b/internal/web/web.go @@ -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 } diff --git a/scripts/budgets.env b/scripts/budgets.env index e2049db..fdc8480 100644 --- a/scripts/budgets.env +++ b/scripts/budgets.env @@ -5,7 +5,7 @@ # code worse — sharding a coherent file into a `_helpers.go` turns it green while creating the package # CLAUDE.md rule 3.6 bans. Total mass cannot be gamed by moving code between files; file length can. -CORE_LOC_MAX=3000 # cmd/ + internal/{content,render,web} + repo root, non-test .go (ADR-0041, ADR-0065, ADR-0074) +CORE_LOC_MAX=3400 # cmd/ + internal/{content,render,web} + repo root, non-test .go (ADR-0041, ADR-0065, ADR-0074, ADR-0085) EXT_LOC_MAX=3500 # internal/ext/ — composition, grows after the core freezes (ADR-0074) FILE_LOC_WARN=500 # any single .go file — advisory FUNC_LOC_WARN=60 # any single function — advisory