Make Server take a variadic array of Options and store config in ServerConfig (#72)

[harrydayexe]

Closes #72

Note

This pull request was written by rain running Claude Code. It has not been reviewed by a human. CI and an automated review pass run before it is handed over.

It targets v3-changes rather than the default branch: 72-make-server-take-a-variadic-array-of-options-and-store-config-in-serverconfig was already linked to the issue, and its history says it was cut from v3-changes.

Issue

#72: Make Server take a variadic array of Options and store config in ServerConfig

What the agent reports

Pushed. Four commits on the branch, baselined on v3-changes.

What changed

pkg/config

• ServerConfig is now the resolved server configuration — BlogRoot, Port, Host, Logger, CacheControlTTL, HealthChecks, AssetsDir, TemplateDir, plus Middleware, GeneratorOpts, RendererOpts. It no longer carries Server/Gen/TemplateDir/RendererOpts option slices.
• BaseServerOption → ServerOption, matching GeneratorOption/RendererOption/WatcherOption.
• New plumbing so everything the old struct fields carried can travel in the variadic list: GeneratorOption.AsServerOption(), RendererOption.AsServerOption(), and a new TemplateDir config type with WithTemplateDir(fs.FS) (+ AsOption). BaseOption.AsServerOption() is unchanged.

pkg/server

• func New(posts fs.FS, opts ...config.ServerOption) (*Server, error); Server embeds config.ServerConfig instead of seven separate config types and three unexported deferred-init fields. Field promotion means srv.Port, srv.Logger.Logger, srv.HealthChecks.Enabled etc. still read the same way.

cli/internal/server — builds a []config.ServerOption; runServe now takes (ctx, postsPath, posts, watch, opts...). I dropped the duplicate blog-root generator option there (and in one test): initialize assigns s.generator.BlogRoot = s.BlogRoot unconditionally, so applying it twice was a no-op.

Tests — all existing call sites migrated (pkg/server, cli/internal/server, integration). New pkg/server/options_test.go covers defaults, every option resolving into the embedded ServerConfig, health-checks deferring generator construction, and generator/renderer/template-dir options reaching the internal generator and renderer (the last renders a custom template through a forwarded WithFuncs entry). These fail without the change — the APIs they use don't exist on the old signature.

Docs — pkg/server/doc.go (new "# Configuration" section), pkg/config/doc.go, the option godoc examples, and README examples all rewritten for the variadic form; added a short serve example to the library section of README.

Verification

go vet ./..., go test ./... and go test -race ./... for both . and cli; go vet ./... for integration; gofmt -l clean; go mod tidy a no-op in all three modules. I did not run the integration suite (just test-integration) — it needs Docker; those tests compile and were updated. A CLI smoke run was blocked by a permission prompt, so end-to-end serve coverage rests on the CLI unit tests (index, post, blog-root, watch-reload, feeds).

For review

• The rename BaseServerOption → ServerOption and the ServerConfig redefinition are the breaking parts; commit 1 is marked feat(server)! with a BREAKING CHANGE: footer. Worth confirming this is the naming you want for v3 given the issue wrote config.ServerOptions.
• New appends the server's own logger to ServerConfig.GeneratorOpts, so that slice holds one more option than the caller passed.
• GeneratorOption.AsServerOption shadows the AsServerOption promoted from its embedded BaseOption; config.WithLogger(l).AsGeneratorOption().AsServerOption() therefore routes to the generator, while config.WithLogger(l).AsServerOption() configures the server (which forwards it anyway).

Changelog (#103)

📚 Documentation

• document the variadic server options API

🧪 Tests

• (server) cover variadic option plumbing
• (server) drop redundant generator blog root option
• (server) cover blog root forwarding to the generator

❓ Uncategorised!

• feat(server)!: take variadic options and store resolved config

[harrydayexe]

Automated review

I read the full diff, the surrounding code in pkg/config, pkg/server, pkg/generator, and the CLI, and ran go vet + go test -race -count=1 on the root, cli, and integration modules (all pass).

The core change is sound. All eleven ServerOption function fields are handled in New's else if chain with none silently dropped, defaults (Port: 8080, CacheControlTTL: time.Hour, slog.Default(), templates.Default) are preserved, GeneratorOpts is built into a slice the server owns so the logger append can't alias a caller's slice, and the CLI's option list is a complete one-for-one translation of the old cfg.Server/cfg.Gen/cfg.TemplateDir assignments. The pre-existing s.generator.BlogRoot = s.BlogRoot override and the "Using custom templates" log on the default template FS are both unchanged by this PR.

One finding:

1. cli/internal/server/server.go:96-98 — test-coverage gap for the blog-root forwarding the CLI now depends on. The CLI dropped cfg.Gen = append(cfg.Gen, config.WithBlogRoot(blogRoot).AsGeneratorOption()) and justified it in a comment ("The server forwards its resolved blog root to the generator"). That forwarding is real — pkg/server/server.go:184 does s.generator.BlogRoot = s.BlogRoot after generator.New, and Generate feeds it into parserCfg.BlogRoot — so the behaviour is correct today. But nothing asserts it. cli/internal/server/command_test.go:92 (TestRunServe_BlogRoot) only checks routing (GET /blog/ → 200, GET / → 404), which is driven by the server's BlogRoot in Handler, not the generator's; pkg/server/options_test.go:71 and assets_test.go:103 likewise only exercise the server side. Why it matters: s.generator.BlogRoot = s.BlogRoot reads as redundant next to the option-application loop and is a plausible future cleanup target — if it goes, --blog-root silently emits wrong in-page link and image paths while every test still passes. Concrete fix: in pkg/server/options_test.go, extend TestNew_GeneratorOptionForwarded (or TestNew_OptionsResolveIntoServerConfig, which already passes config.WithBlogRoot("/blog/").AsServerOption()) with an assertion that srv.generator.BlogRoot == "/blog/".

Agent response

Pushed as c0c6fbd.

Review points

1. cli/internal/server/server.go:96-98 — test-coverage gap for blog-root forwarding — FIXED.

The finding was accurate: nothing asserted that s.generator.BlogRoot = s.BlogRoot (pkg/server/server.go:184) actually happens. I went beyond the suggested one-line assertion, because a field-equality check alone doesn't document why the forwarding matters — it reads as just as redundant as the line it guards.

Two changes, both in pkg/server/options_test.go:

• TestNew_OptionsResolveIntoServerConfig now asserts srv.generator.BlogRoot == "/blog/", with a comment noting the generator has no blog root of its own.
• New TestNew_BlogRootForwardedToGenerator covers the user-visible behaviour: a post containing ![diagram](diagram.png) is served under --root-path-equivalent options with a template that emits {{ .BlogRoot }}|{{ .Post.HTMLContent }}, and the test asserts the page data carries /blog/ and the image is rewritten to src="/blog/images/diagram.png". Its doc comment states the CLI dependency explicitly, so a future cleanup pass sees the consequence rather than just a failing equality.

Mutation-checked: with s.generator.BlogRoot = s.BlogRoot deleted, both tests fail, and the new one reports /|<p><img src="/images/diagram.png" ... — the exact breakage the reviewer described. Line restored afterwards; git diff on server.go is empty, so this commit is test-only.

I left the CLI comment at cli/internal/server/server.go:96-98 as-is — it describes behaviour that is correct and now covered.

Verification

• go vet ./... — clean on root, cli, integration.
• go test -race -count=1 ./... — passes on root, cli, and integration.
• gofmt -l . — clean.

---

Posted by rain. Both halves were written by Claude Code; treat them as a starting point for human review, not a substitute for it.