Admin-gated Settings: runtime model selection, and is_admin enforced for the first time #90

Merged
steve merged 2 commits from feat/settings-admin into main 2026-07-22 02:53:55 +00:00
24 changed files with 1316 additions and 103 deletions
+19
View File
@@ -163,6 +163,25 @@ Agent config: `OLLAMA_CLOUD_API_KEY`, `PANSY_AGENT_MODEL` (default
strings pass verbatim to `majordomo.Parse`, so a comma-separated spec gives
failover for free — don't parse that grammar in pansy.
The model and enabled flag are also **admin-editable at runtime** in Settings
(#79); the env vars are just defaults (precedence: DB setting → env → default).
Two things this makes load-bearing:
- **The live Runner is hot-swapped, not built once.** It sits behind an
`atomic.Pointer` in `internal/api` (`agentHolder`), and its routes are
registered *unconditionally* with a nil-check on `agent.get()`. Do NOT go back
to registering the chat routes only when a key is present — a settings change
has to be able to turn the assistant on without a restart, which a missing
route can't. `/capabilities` reads the pointer, so it reflects the live state.
- **`OLLAMA_CLOUD_API_KEY` stays in the environment, never the DB.** Model
selection is a setting; the key is not. A secret in `instance_settings` lands
in every backup and in the undo history's blast radius. The Settings API
reports whether a key is *present*, never its value.
- The "how to turn a spec into a model" knowledge lives once in
`internal/agentmodel`, imported by both `agent` (to run) and `service` (to
validate a spec before storing it). It can't live in `agent``agent` imports
`service`, so a `service``agent` import would cycle.
`majordomo` is a **real dependency** now, resolved from the Gitea instance as a
pseudo-version. There is no `replace` directive and there must not be one: a
`replace` pointing at `../majordomo` builds on your laptop and breaks the Docker
+4 -2
View File
@@ -9,8 +9,9 @@ Work is tracked in Gitea issues; the tracking epic links every piece in dependen
- **Placement model:** freeform plops (not a square-foot grid), scaled by real plant spacing. Grid snapping may come later as a toggle.
- **Spacing is a plant-to-plant rule, so bed edges get half of it.** A bed edge is not a competitor for soil, light or water, so the outer row owes it half the spacing rather than a full one. `FillRegion` centres its lattice accordingly, and lets a plop — a *clump* three spacings across — cross the edge by up to half a spacing so its outermost plants land at that half-spacing. The rule, the square-foot-chart arithmetic behind it, and the failure mode it prevents are written out once in `hexCenters`; #75 is what getting it wrong looked like.
- **Stack:** Go 1.26.x backend, module `gitea.stevedudenhoeffer.com/steve/pansy`; React + TypeScript + Vite + Tailwind frontend, production build embedded via `embed.FS` → one static binary (`CGO_ENABLED=0`).
- **Users:** multi-user with ownership. Users own gardens; a garden can be shared with other users as viewer (read) or editor (edit content). Owner additionally shares/deletes.
- **Users:** multi-user with ownership. Users own gardens; a garden can be shared with other users as viewer (read) or editor (edit content). Owner additionally shares/deletes. The first registered user is `is_admin` (set race-free inside the INSERT); admin gates instance-wide Settings (`requireAdmin`), the only thing that reads that flag.
- **Auth:** OIDC-first (Authentik is the primary IdP), local argon2id passwords as an optional fallback.
- **Instance settings (#79):** admin-editable, instance-wide config in a single-row `instance_settings` table — pansy's first DB-stored *instance* state (everything else hangs off a garden/object). Today it holds the agent model + on/off; **secrets never move here**`OLLAMA_CLOUD_API_KEY` stays in the env so it doesn't land in backups or the undo history. Precedence: Settings value → env → default. The live agent Runner sits behind an `atomic.Pointer` in the API layer (`agentHolder`) with its routes always registered, so a settings change swaps it with no restart and no race against in-flight requests; `/capabilities` reads that pointer, so it reports what's live rather than what was configured at boot. The model/registry knowledge lives in one leaf package (`internal/agentmodel`) that both the runner and the settings validator import — agent imports service, so it can live in neither.
- **Agentic future:** integration with majordomo/executus via typed Go tools (`llm.DefineTool[Args]`) wrapping the same service layer the REST API uses — not MCP/OpenAPI.
## Domain model
@@ -70,7 +71,8 @@ GET,POST /gardens/:id/journal PATCH,DELETE /journal/:id (editor writes
GET /gardens/:id/journal/counts ← entries per object, for the "has notes" indicator
POST /agent/chat ← SSE: step events, then the finished turn (editor only)
GET,DELETE /gardens/:id/agent/history (the actor's own thread)
GET /capabilities ← what this instance can do, so the UI offers only what works
GET /capabilities ← what this instance can do RIGHT NOW (tracks the live agent, not just config)
GET,PATCH /settings ← instance-wide config (admin only): agent model + on/off
GET,POST /gardens/:id/shares PATCH,DELETE /gardens/:id/shares/:userId (invite by email)
```
+6 -4
View File
@@ -67,13 +67,15 @@ The garden assistant reads three more. Setting none of them leaves the assistant
| Variable | Default | Description |
| ----------------------- | ------------------------------ | --------------------------------------------------------------------------- |
| `OLLAMA_CLOUD_API_KEY` | *(empty)* | Ollama Cloud API key. Without it the assistant is disabled, not broken — the chat routes simply aren't registered. |
| `PANSY_AGENT_MODEL` | `ollama-cloud/glm-5.2:cloud` | Model spec, passed verbatim to `majordomo.Parse` — a comma-separated list is a failover chain, e.g. `ollama-cloud/glm-5.2:cloud,ollama-cloud/kimi-k2.6:cloud`. |
| `PANSY_AGENT_ENABLED` | on when a key is present | Turns the assistant off without removing the key. |
| `OLLAMA_CLOUD_API_KEY` | *(empty)* | Ollama Cloud API key. Without it the assistant is off, not broken. This is the one agent value that stays in the environment — it is **never** stored in the database or editable in Settings. |
| `PANSY_AGENT_MODEL` | `ollama-cloud/glm-5.2:cloud` | Default model spec, passed verbatim to `majordomo.Parse` — a comma-separated list is a failover chain, e.g. `ollama-cloud/glm-5.2:cloud,ollama-cloud/kimi-k2.6:cloud`. An admin can override this per-instance in **Settings** without a redeploy; a blank Settings value inherits this. |
| `PANSY_AGENT_ENABLED` | on when a key is present | Default on/off for the assistant. Also overridable in Settings (which can inherit this default). |
The model and enabled flag can be changed at runtime by an admin under **Settings** (the gear appears in the nav for admins) — the change swaps the live assistant with no restart. The env vars above are the defaults an untouched instance uses, and the API key is intentionally not among the runtime-editable settings: a secret in the database would land in every backup. Precedence for the model and enabled flag is **Settings value, if set → env var → built-in default**.
The assistant acts without asking first, which is only reasonable because every turn is one undoable change set — see the History panel in the editor.
**If you set the key and the assistant still doesn't appear**, check that the variable reaches the *container*, not just your orchestrator's stack config — Compose needs it listed under the service's `environment:`. pansy logs `garden assistant disabled` at startup with which of the three conditions failed, so the answer is in the first few lines of the log.
**If you set the key and the assistant still doesn't appear**, check that the variable reaches the *container*, not just your orchestrator's stack config — Compose needs it listed under the service's `environment:`. pansy logs why the assistant is off at startup, and Settings shows the same status (a key present, the resolved model, and whether it's actually running).
Local email/password auth is live (`POST /api/v1/auth/register`, `/auth/login`, `/auth/logout`, `GET /auth/me`, `GET /auth/providers`); the session is an HttpOnly cookie (`Secure` when `PANSY_BASE_URL` is https). The first account registered becomes admin, and it may register even when `PANSY_REGISTRATION=closed` to bootstrap the instance.
+15 -23
View File
@@ -10,12 +10,10 @@ import (
"strings"
"time"
"gitea.stevedudenhoeffer.com/steve/majordomo"
"gitea.stevedudenhoeffer.com/steve/majordomo/agent"
"gitea.stevedudenhoeffer.com/steve/majordomo/llm"
"gitea.stevedudenhoeffer.com/steve/majordomo/provider/ollama"
"gitea.stevedudenhoeffer.com/steve/pansy/internal/config"
"gitea.stevedudenhoeffer.com/steve/pansy/internal/agentmodel"
"gitea.stevedudenhoeffer.com/steve/pansy/internal/domain"
"gitea.stevedudenhoeffer.com/steve/pansy/internal/service"
)
@@ -41,29 +39,23 @@ type Runner struct {
model llm.Model
}
// NewRunner resolves the configured model and returns a Runner, or an error if
// the assistant can't be offered. Callers should treat an error as "no
// assistant" rather than a startup failure — an instance with no key must still
// serve the app.
func NewRunner(svc *service.Service, cfg *config.Config) (*Runner, error) {
if !cfg.Agent.Ready() {
// NewRunner resolves modelSpec against pansy's registry and returns a Runner, or
// an error if the assistant can't be offered. Callers should treat an error as
// "no assistant" rather than a startup failure — an instance with no key must
// still serve the app.
//
// It takes the key and spec explicitly rather than a *config.Config so the same
// constructor serves both boot (from env) and a runtime settings change (from
// the DB) — the Runner has no idea which one configured it.
func NewRunner(svc *service.Service, apiKey, modelSpec string) (*Runner, error) {
if apiKey == "" {
return nil, errors.New("agent: not configured")
}
// A private registry, not the package-level default: pansy passes the key it
// was configured with rather than depending on ambient environment, and
// majordomo's own ollama-cloud preset reads OLLAMA_API_KEY while pansy (like
// gadfly) is configured with OLLAMA_CLOUD_API_KEY. Registering the provider
// explicitly makes that bridge visible instead of a mysterious empty token.
reg := majordomo.New()
reg.RegisterProvider(ollama.Cloud(ollama.WithToken(cfg.Agent.OllamaCloudAPIKey)))
// The spec goes to Parse VERBATIM. The grammar — including comma-separated
// failover chains — is majordomo's, and re-implementing any of it here would
// only mean two places to update when it grows.
model, err := reg.Parse(cfg.Agent.Model)
// agentmodel.Resolve already rejects an empty/blank spec, so don't duplicate
// that guard here — one place decides what a valid spec is.
model, err := agentmodel.Resolve(apiKey, modelSpec)
if err != nil {
return nil, fmt.Errorf("agent: resolve model %q: %w", cfg.Agent.Model, err)
return nil, err
}
return &Runner{svc: svc, model: model}, nil
}
+11 -12
View File
@@ -13,7 +13,6 @@ import (
"gitea.stevedudenhoeffer.com/steve/majordomo/llm"
"gitea.stevedudenhoeffer.com/steve/majordomo/provider/fake"
"gitea.stevedudenhoeffer.com/steve/pansy/internal/config"
"gitea.stevedudenhoeffer.com/steve/pansy/internal/domain"
"gitea.stevedudenhoeffer.com/steve/pansy/internal/service"
)
@@ -232,24 +231,24 @@ func TestReadOnlyTurnWritesNoChangeSet(t *testing.T) {
}
}
// TestNewRunnerNeedsConfiguration — an instance with no key must not get a
// half-built runner; the caller treats the error as "no assistant" and carries on.
// TestNewRunnerNeedsConfiguration — an instance with no key or no model must not
// get a half-built runner; the caller treats the error as "no assistant" and
// carries on. (Whether the assistant is ENABLED is resolved before NewRunner is
// reached, so it isn't NewRunner's concern any more.)
func TestNewRunnerNeedsConfiguration(t *testing.T) {
svc, _ := newAgentTestService(t)
for _, cfg := range []*config.Config{
{Agent: config.AgentConfig{Enabled: false, OllamaCloudAPIKey: "k", Model: "ollama-cloud/x"}},
{Agent: config.AgentConfig{Enabled: true, OllamaCloudAPIKey: "", Model: "ollama-cloud/x"}},
{Agent: config.AgentConfig{Enabled: true, OllamaCloudAPIKey: "k", Model: ""}},
for _, tc := range []struct{ key, model string }{
{"", "ollama-cloud/x"},
{"k", ""},
{"k", " "},
} {
if _, err := NewRunner(svc, cfg); err == nil {
t.Errorf("NewRunner accepted %+v", cfg.Agent)
if _, err := NewRunner(svc, tc.key, tc.model); err == nil {
t.Errorf("NewRunner accepted key=%q model=%q", tc.key, tc.model)
}
}
// A model spec naming a provider that doesn't exist is a configuration
// error, not a panic at first use.
if _, err := NewRunner(svc, &config.Config{Agent: config.AgentConfig{
Enabled: true, OllamaCloudAPIKey: "k", Model: "nonesuch/model",
}}); err == nil {
if _, err := NewRunner(svc, "k", "nonesuch/model"); err == nil {
t.Error("NewRunner accepted an unresolvable model spec")
}
}
+57
View File
@@ -0,0 +1,57 @@
// Package agentmodel holds the ONE place that knows how pansy turns a model spec
// into a majordomo model: which provider to register and under which token.
//
// It exists as a leaf so both internal/agent (which builds the run loop) and
// internal/service (which validates a spec before storing it as a setting) can
// share that knowledge without an import cycle — agent imports service, so the
// shared bit can live in neither of them.
package agentmodel
import (
"errors"
"fmt"
"strings"
"gitea.stevedudenhoeffer.com/steve/majordomo"
"gitea.stevedudenhoeffer.com/steve/majordomo/llm"
"gitea.stevedudenhoeffer.com/steve/majordomo/provider/ollama"
)
// registry builds the private majordomo registry pansy uses.
//
// Private, not the package-level default: pansy passes the key it was configured
// with rather than depending on ambient environment, and majordomo's own
// ollama-cloud preset reads OLLAMA_API_KEY while pansy (like gadfly) is
// configured with OLLAMA_CLOUD_API_KEY. Registering the provider explicitly
// makes that bridge visible instead of a mysterious empty token.
func registry(apiKey string) *majordomo.Registry {
reg := majordomo.New()
reg.RegisterProvider(ollama.Cloud(ollama.WithToken(apiKey)))
return reg
}
// Resolve parses a model spec against pansy's registry into a live model. The
// spec goes to Parse VERBATIM — the grammar, including comma-separated failover
// chains, is majordomo's, and re-implementing any of it here would only mean two
// places to update when it grows.
func Resolve(apiKey, spec string) (llm.Model, error) {
if strings.TrimSpace(spec) == "" {
return nil, errors.New("agentmodel: empty model spec")
}
m, err := registry(apiKey).Parse(spec)
if err != nil {
return nil, fmt.Errorf("agentmodel: resolve %q: %w", spec, err)
}
return m, nil
}
// Validate reports whether a spec resolves, without building anything the caller
// keeps — the cheap, deterministic check a settings save runs to reject a typo.
//
// It does NOT make a live call, so it needs no working key and won't catch a
// model that is merely absent upstream; that surfaces on first use. Parse
// resolving (known provider, well-formed spec) is the half worth doing eagerly.
func Validate(apiKey, spec string) error {
_, err := Resolve(apiKey, spec)
return err
}
+35
View File
@@ -0,0 +1,35 @@
package agentmodel
import "testing"
// TestValidate is what the settings PATCH relies on to reject a typo at save
// time rather than on the next chat turn.
func TestValidate(t *testing.T) {
if err := Validate("k", "ollama-cloud/glm-5.2:cloud"); err != nil {
t.Errorf("valid spec rejected: %v", err)
}
// No key needed to validate — Parse resolves the provider, it doesn't call it.
if err := Validate("", "ollama-cloud/glm-5.2:cloud"); err != nil {
t.Errorf("valid spec rejected without a key: %v", err)
}
// A comma-separated failover chain is majordomo grammar and must resolve.
if err := Validate("k", "ollama-cloud/glm-5.2:cloud,ollama-cloud/kimi-k2.6:cloud"); err != nil {
t.Errorf("failover chain rejected: %v", err)
}
for _, bad := range []string{"", " ", "nonesuch/model"} {
if err := Validate("k", bad); err == nil {
t.Errorf("Validate accepted %q", bad)
}
}
}
// TestResolveReturnsAModel confirms a good spec yields a usable model handle.
func TestResolveReturnsAModel(t *testing.T) {
m, err := Resolve("k", "ollama-cloud/glm-5.2:cloud")
if err != nil {
t.Fatalf("resolve: %v", err)
}
if m == nil {
t.Fatal("resolve returned a nil model with no error")
}
}
+11 -1
View File
@@ -56,6 +56,16 @@ type stepEvent struct {
}
func (h *handlers) agentChat(c *gin.Context) {
// The route is always registered, so the assistant being off is a runtime
// state, not a missing route: answer it plainly rather than 404ing a path
// that exists. Loaded once here so a settings-driven swap mid-request can't
// make it flip between the guard and the Run call.
runner := h.agent.get()
if runner == nil {
writeAPIError(c, http.StatusServiceUnavailable, "AGENT_DISABLED", "the garden assistant isn't enabled on this instance")
return
}
var req chatRequest
if err := c.ShouldBindJSON(&req); err != nil {
writeAPIError(c, http.StatusBadRequest, "INVALID_INPUT", "a gardenId and a message are required")
@@ -78,7 +88,7 @@ func (h *handlers) agentChat(c *gin.Context) {
stopBeat := stream.keepAlive(keepAliveInterval)
defer stopBeat()
turn, err := h.agent.Run(c.Request.Context(), actor.ID, req.GardenID, req.Message,
turn, err := runner.Run(c.Request.Context(), actor.ID, req.GardenID, req.Message,
replayHistory(history),
func(s mdagent.Step) {
send(chatEvent{Step: &stepEvent{Index: s.Index, Tools: toolNames(s)}})
+89
View File
@@ -0,0 +1,89 @@
package api
import (
"context"
"log/slog"
"sync"
"sync/atomic"
"gitea.stevedudenhoeffer.com/steve/pansy/internal/agent"
"gitea.stevedudenhoeffer.com/steve/pansy/internal/service"
)
// agentHolder owns the live assistant Runner and lets an admin swap it at
// runtime when the model settings change (#79).
//
// The Runner used to be a plain handlers field set once at boot, with the chat
// routes registered only when it existed. That made the assistant permanently
// whatever the environment said at startup. Now the routes are always
// registered and the Runner lives behind an atomic pointer, so a settings change
// can turn the assistant on, off, or onto a different model without a restart —
// and without a data race against in-flight requests reading the pointer.
//
// A nil pointer means "no assistant right now"; handlers nil-check get() rather
// than assuming a Runner is present.
type agentHolder struct {
svc *service.Service
ptr atomic.Pointer[agent.Runner]
// rebuildMu serializes rebuilds so two concurrent settings saves can't
// interleave into a torn "resolve A, resolve B, store A, store B" swap. The
// read path (get) stays lock-free on the atomic pointer.
rebuildMu sync.Mutex
}
// newAgentHolder builds the holder and resolves the initial Runner from whatever
// the settings + environment currently say. A resolution failure is logged and
// left as "no assistant", never fatal: a garden planner must still boot.
func newAgentHolder(ctx context.Context, svc *service.Service) *agentHolder {
h := &agentHolder{svc: svc}
h.rebuild(ctx)
return h
}
// get returns the current Runner, or nil if the assistant is off.
func (h *agentHolder) get() *agent.Runner { return h.ptr.Load() }
// rebuild resolves the effective agent configuration and swaps the Runner to
// match: a new one when the assistant should be on, nil when it shouldn't. It is
// safe to call at boot and from a settings save; concurrent calls serialize.
//
// It logs what it did rather than returning an error, because every caller wants
// the same thing — best-effort apply, keep serving either way — and a settings
// save must not fail just because the new model won't resolve. The save already
// validated the spec; a rebuild failure here means the environment changed under
// it, and "assistant off, with a reason in the log" is the right outcome.
func (h *agentHolder) rebuild(ctx context.Context) {
h.rebuildMu.Lock()
defer h.rebuildMu.Unlock()
eff, err := h.svc.EffectiveAgent(ctx)
if err != nil {
// EffectiveAgent errors only if the settings row can't be read — a DB fault,
// and a very transient one when it happens right after a settings write. We
// keep the current Runner rather than tear down a working assistant on a
// blip: the state is already persisted, so the next rebuild (any later save,
// or a restart) reconciles it. Loud, because a persistent failure here means
// the live assistant no longer matches stored settings.
slog.Error("api: could not resolve agent settings; leaving the assistant as-is", "error", err)
return
}
if !eff.Ready() {
if h.ptr.Swap(nil) != nil {
slog.Info("api: garden assistant turned off",
"enabled", eff.Enabled, "hasKey", eff.APIKey != "", "hasModel", eff.Model != "")
}
return
}
runner, err := agent.NewRunner(h.svc, eff.APIKey, eff.Model)
if err != nil {
// Configured but unusable. Turn the assistant off rather than leaving a
// stale Runner on the old model — an admin who just pointed it at a broken
// spec should see it stop, not silently keep answering on the previous one.
slog.Error("api: garden assistant disabled (model won't resolve)", "error", err, "model", eff.Model)
h.ptr.Store(nil)
return
}
h.ptr.Store(runner)
slog.Info("api: garden assistant ready", "model", eff.Model)
}
+25 -9
View File
@@ -6,24 +6,40 @@ import (
"testing"
)
// TestAgentRoutesAbsentWithoutAKey — an instance with no API key must start,
// serve the app, and simply not offer the assistant. The routes aren't
// registered at all, so this is a 404 rather than a handler that apologizes:
// the same shape as OIDC when unconfigured.
func TestAgentRoutesAbsentWithoutAKey(t *testing.T) {
// TestAgentDisabledWithoutAKey — an instance with no API key must start, serve
// the app, and not offer the assistant.
//
// The contract CHANGED with #79: the chat route is now always registered (so a
// settings change can turn the assistant on without a restart), so "off" is a
// runtime 503 rather than a missing route. capabilities reports agent:false, and
// the frontend keys the chat tab off that — so a user never reaches the 503.
func TestAgentDisabledWithoutAKey(t *testing.T) {
r := authEngine(t, localCfg()) // localCfg has no agent configuration
cookie := registerAndCookie(t, r, "[email protected]")
gid := createGardenAPI(t, r, cookie, "G")
// Chat is refused, plainly, because there is no Runner to run.
w := doJSON(t, r, http.MethodPost, "/api/v1/agent/chat",
map[string]any{"gardenId": gid, "message": "plant garlic"}, cookie)
if w.Code != http.StatusNotFound {
t.Errorf("chat without a key: status %d, want 404", w.Code)
if w.Code != http.StatusServiceUnavailable {
t.Errorf("chat without a key: status %d, want 503", w.Code)
}
// Capabilities advertises the assistant as unavailable, which is what the UI
// actually consults.
w = doJSON(t, r, http.MethodGet, "/api/v1/capabilities", nil, cookie)
if w.Code != http.StatusOK {
t.Fatalf("capabilities: status %d", w.Code)
}
if agent, _ := decodeMap(t, w.Body.Bytes())["agent"].(bool); agent {
t.Error("capabilities reported agent:true with no key")
}
// History is just stored data behind the ordinary garden-role check, so it
// reads fine (empty) whether or not a Runner exists — it isn't gated on one.
path := "/api/v1/gardens/" + strconv.FormatInt(gid, 10) + "/agent/history"
if w := doJSON(t, r, http.MethodGet, path, nil, cookie); w.Code != http.StatusNotFound {
t.Errorf("history without a key: status %d, want 404", w.Code)
if w := doJSON(t, r, http.MethodGet, path, nil, cookie); w.Code != http.StatusOK {
t.Errorf("history without a key: status %d, want 200 (it's data, not the model)", w.Code)
}
// And the rest of the app is entirely unaffected.
+35 -38
View File
@@ -6,13 +6,13 @@
package api
import (
"context"
"log/slog"
"net/http"
"github.com/gin-gonic/gin"
sloggin "github.com/samber/slog-gin"
"gitea.stevedudenhoeffer.com/steve/pansy/internal/agent"
"gitea.stevedudenhoeffer.com/steve/pansy/internal/config"
"gitea.stevedudenhoeffer.com/steve/pansy/internal/service"
)
@@ -23,9 +23,12 @@ type handlers struct {
cfg *config.Config
svc *service.Service
oidc *oidcClient // nil unless OIDC is configured (see config.OIDCReady)
// agent is nil unless the assistant is configured; the chat routes are only
// registered when it isn't, so a handler never has to check.
agent *agent.Runner
// agent holds the live Runner behind an atomic pointer. Unlike oidc it is
// never nil — the holder is always present and its Runner may be nil when the
// assistant is off. The chat routes are registered unconditionally and
// nil-check agent.get(), so a settings change can turn the assistant on or off
// at runtime (#79) rather than only at boot.
agent *agentHolder
}
// New builds the gin engine with the standard middleware stack and registers the
@@ -126,37 +129,30 @@ func New(cfg *config.Config, svc *service.Service) *gin.Engine {
plantings.PATCH("/:id", h.updatePlanting)
plantings.DELETE("/:id", h.deletePlanting)
// The garden assistant, registered only when it can actually be offered —
// the same shape as OIDC. An instance with no API key serves the app
// normally and simply doesn't have these routes.
if !cfg.Agent.Ready() {
// Say WHY, at startup, in the logs an operator is already looking at.
// Someone who set the key and sees no assistant otherwise has nothing to
// check — and "is the variable reaching the container?" is exactly the
// question they need answered.
slog.Info("api: garden assistant disabled",
"enabled", cfg.Agent.Enabled,
"hasApiKey", cfg.Agent.OllamaCloudAPIKey != "",
"model", cfg.Agent.Model,
"hint", "needs OLLAMA_CLOUD_API_KEY set in the container's environment (not just the stack's)")
}
if cfg.Agent.Ready() {
runner, err := agent.NewRunner(svc, cfg)
if err != nil {
// Configured but unusable (an unresolvable model spec, say). Log it and
// carry on without the assistant rather than refusing to start: a
// garden planner that won't boot because of a chat feature is worse
// than one without chat.
slog.Error("api: garden assistant disabled", "error", err)
} else {
h.agent = runner
agentGroup := v1.Group("/agent", h.requireAuth())
agentGroup.POST("/chat", h.agentChat)
gardens.GET("/:id/agent/history", h.getAgentHistory)
gardens.DELETE("/:id/agent/history", h.deleteAgentHistory)
slog.Info("api: garden assistant enabled", "model", cfg.Agent.Model)
}
// The garden assistant. Its routes are registered UNCONDITIONALLY and the live
// Runner sits behind an atomic pointer in the holder, so a settings change can
// turn the assistant on or off at runtime (#79). Each handler nil-checks
// agent.get(); a chat request while the assistant is off gets a clean 503
// (AGENT_DISABLED), not a panic and not a missing route.
//
// The holder resolves its initial Runner from settings + environment at
// construction. If the key never reaches the container, the assistant is off
// and the reason is logged below — the same operability need #72 added.
h.agent = newAgentHolder(context.Background(), svc)
if cfg.Agent.OllamaCloudAPIKey == "" {
slog.Info("api: garden assistant has no API key",
"hint", "set OLLAMA_CLOUD_API_KEY in the container's environment (not just the stack's); the model can be chosen in Settings")
}
agentGroup := v1.Group("/agent", h.requireAuth())
agentGroup.POST("/chat", h.agentChat)
gardens.GET("/:id/agent/history", h.getAgentHistory)
gardens.DELETE("/:id/agent/history", h.deleteAgentHistory)
// Instance settings: admin-only, and the first thing to enforce is_admin.
// requireAdmin runs after requireAuth (it reads the actor requireAuth stored).
settings := v1.Group("/settings", h.requireAuth(), h.requireAdmin())
settings.GET("", h.getSettings)
settings.PATCH("", h.updateSettings)
// Undo. A change set is addressed by its own id; the service resolves the
// owning garden for the permission check, same as objects and plantings.
@@ -198,11 +194,12 @@ func New(cfg *config.Config, svc *service.Service) *gin.Engine {
// capabilities reports what this instance can actually do, so the UI offers only
// what works.
//
// It reports whether the runner BUILT, not whether it was configured: a
// configured-but-unresolvable model leaves the routes unregistered, and saying
// "yes" there would offer a chat tab whose first message 404s.
// It reports whether the assistant is live RIGHT NOW, not merely configured:
// the chat routes always exist, but a request while the Runner is nil is refused,
// so offering the tab must track the live Runner. Reading agent.get() (an atomic
// load) means this reflects a settings-driven swap on the very next poll.
func (h *handlers) capabilities(c *gin.Context) {
c.JSON(http.StatusOK, gin.H{"agent": h.agent != nil})
c.JSON(http.StatusOK, gin.H{"agent": h.agent.get() != nil})
}
// healthz is a liveness probe: always returns {"ok": true} when the server is up.
+143
View File
@@ -0,0 +1,143 @@
package api
import (
"context"
"encoding/json"
"errors"
"net/http"
"github.com/gin-gonic/gin"
"gitea.stevedudenhoeffer.com/steve/pansy/internal/domain"
"gitea.stevedudenhoeffer.com/steve/pansy/internal/service"
)
// Instance settings (#79): admin-only, instance-wide. The authoritative admin
// check is in the service; requireAdmin here is a cheap early 403 that also
// keeps the route group readable.
// requireAdmin rejects a non-admin actor. It runs after requireAuth, so the
// actor is already resolved and carries IsAdmin — no extra query. Returns 403
// (not 404): a logged-in user knows settings exist, they just may not touch them.
func (h *handlers) requireAdmin() gin.HandlerFunc {
return func(c *gin.Context) {
if !mustActor(c).IsAdmin {
writeAPIError(c, http.StatusForbidden, "FORBIDDEN", "admin access required")
c.Abort()
return
}
c.Next()
}
}
// settingsResponse is what GET/PATCH /settings return. It carries the stored
// settings plus a read-only view of what's resolved and live, so the UI can show
// "inheriting ollama-cloud/glm-5.2:cloud from the environment" and whether a key
// is present — without ever exposing the key itself.
type settingsResponse struct {
Settings *domain.InstanceSettings `json:"settings"`
// Effective is the configuration actually in force after layering settings
// over the environment.
Effective effectiveView `json:"effective"`
}
type effectiveView struct {
Model string `json:"model"`
Enabled bool `json:"enabled"`
// HasApiKey reports whether OLLAMA_CLOUD_API_KEY is set. The key itself is
// never serialized — an admin may know one exists, not what it is.
HasApiKey bool `json:"hasApiKey"`
// AgentLive is whether the assistant Runner is actually built right now. It
// can be false even when Enabled+HasApiKey are true (an unresolvable model),
// which is exactly the case the UI needs to surface.
AgentLive bool `json:"agentLive"`
}
// settingsPayload builds the response, or an error. It does NOT swallow an
// EffectiveAgent failure into a misleading empty "effective" view — an empty
// view would report no model and no key, which reads as "nothing configured"
// rather than "we couldn't read it". Since EffectiveAgent re-reads the same row
// GetInstanceSettings just returned, a failure here is a genuine DB fault worth
// surfacing as a 500, not papering over.
func (h *handlers) settingsPayload(c *gin.Context, st *domain.InstanceSettings) (settingsResponse, error) {
eff, err := h.svc.EffectiveAgent(c.Request.Context())
Review

🟡 EffectiveAgent re-reads instance_settings row that GetInstanceSettings just fetched

maintainability, performance · flagged by 2 models

  • internal/api/settings.go:63 and internal/api/settings.go:141settingsPayload calls h.svc.EffectiveAgent, which always does a fresh GetInstanceSettings DB read. In getSettings, the same row was just fetched by GetInstanceSettings. In updateSettings, the row was just written and returned, then rebuild calls EffectiveAgent (second read), then settingsPayload calls it again (third read). A single PATCH can hit the same row 3 times. Pass the already-loaded row into `Effe…

🪰 Gadfly · advisory

🟡 **EffectiveAgent re-reads instance_settings row that GetInstanceSettings just fetched** _maintainability, performance · flagged by 2 models_ - `internal/api/settings.go:63` and `internal/api/settings.go:141` — `settingsPayload` calls `h.svc.EffectiveAgent`, which always does a fresh `GetInstanceSettings` DB read. In `getSettings`, the same row was just fetched by `GetInstanceSettings`. In `updateSettings`, the row was just written and returned, then `rebuild` calls `EffectiveAgent` (second read), then `settingsPayload` calls it again (third read). A single PATCH can hit the same row **3 times**. Pass the already-loaded row into `Effe… <sub>🪰 Gadfly · advisory</sub>
if err != nil {
return settingsResponse{}, err
}
return settingsResponse{
Settings: st,
Effective: effectiveView{
Model: eff.Model,
Enabled: eff.Enabled,
HasApiKey: eff.APIKey != "",
AgentLive: h.agent.get() != nil,
},
}, nil
}
func (h *handlers) getSettings(c *gin.Context) {
st, err := h.svc.GetInstanceSettings(c.Request.Context(), mustActor(c).ID)
if err != nil {
writeServiceError(c, err)
return
}
payload, err := h.settingsPayload(c, st)
if err != nil {
writeServiceError(c, err)
return
}
c.JSON(http.StatusOK, payload)
}
// settingsUpdateRequest is the PATCH body. agentModel "" means inherit the env
// var. agentEnabled is json.RawMessage so an explicit null (inherit) is
// distinguishable from an absent field and from true/false.
type settingsUpdateRequest struct {
AgentModel string `json:"agentModel"`
Review

🟡 PATCH /settings binds agentModel as a plain string (full-replacement semantics), so a request that omits it resets the stored model to "inherit" — intentional and documented, but inconsistent with the rest of the codebase's absent=unchanged PATCH convention

correctness · flagged by 1 model

  • internal/api/settings.go:96 (settingsUpdateRequest.AgentModel) — bound as a plain string, not the *string/json.RawMessage "absent vs. explicit" pattern used elsewhere (contrast objectUpdateRequest in internal/api/objects.go:50-65). Since "" is the domain's own sentinel for "inherit from environment" (internal/service/instance_settings.go:60-67, EffectiveAgent at line 103), a PATCH that omits agentModel entirely is indistinguishable from one that explicitly clears it, so `…

🪰 Gadfly · advisory

🟡 **PATCH /settings binds agentModel as a plain string (full-replacement semantics), so a request that omits it resets the stored model to "inherit" — intentional and documented, but inconsistent with the rest of the codebase's absent=unchanged PATCH convention** _correctness · flagged by 1 model_ - `internal/api/settings.go:96` (`settingsUpdateRequest.AgentModel`) — bound as a plain `string`, not the `*string`/`json.RawMessage` "absent vs. explicit" pattern used elsewhere (contrast `objectUpdateRequest` in `internal/api/objects.go:50-65`). Since `""` is the domain's own sentinel for "inherit from environment" (`internal/service/instance_settings.go:60-67`, `EffectiveAgent` at line 103), a PATCH that omits `agentModel` entirely is indistinguishable from one that explicitly clears it, so `… <sub>🪰 Gadfly · advisory</sub>
AgentEnabled json.RawMessage `json:"agentEnabled"`
Version int64 `json:"version" binding:"required"`
}
func (h *handlers) updateSettings(c *gin.Context) {
var req settingsUpdateRequest
if err := c.ShouldBindJSON(&req); err != nil {
writeAPIError(c, http.StatusBadRequest, "INVALID_INPUT", "a current version is required")
return
}
// agentEnabled: absent or null → inherit (nil); true/false → explicit override.
// The shared parseNullable does exactly this three-way decode; present is
// irrelevant here because absent and null both mean "inherit".
enabled, _, err := parseNullable[bool](req.AgentEnabled)
if err != nil {
writeAPIError(c, http.StatusBadRequest, "INVALID_INPUT", "agentEnabled must be true, false, or null")
return
}
st, err := h.svc.UpdateInstanceSettings(c.Request.Context(), mustActor(c).ID, service.InstanceSettingsPatch{
AgentModel: req.AgentModel,
AgentEnabled: enabled,
Version: req.Version,
})
if err != nil {
if errors.Is(err, domain.ErrVersionConflict) {
writeVersionConflict(c, st)
return
}
writeServiceError(c, err)
return
}
// Apply the change to the LIVE assistant. Detached from the request context:
// the write is committed and the rebuild describes it, so a client that hangs
// up now must not leave the running Runner out of step with the stored
// settings. Mirrors the same reasoning as the history-write detachment.
h.agent.rebuild(context.WithoutCancel(c.Request.Context()))
payload, err := h.settingsPayload(c, st)
if err != nil {
writeServiceError(c, err)
return
}
c.JSON(http.StatusOK, payload)
}
+208
View File
@@ -0,0 +1,208 @@
package api
import (
"net/http"
"testing"
"github.com/gin-gonic/gin"
"gitea.stevedudenhoeffer.com/steve/pansy/internal/config"
)
// agentCfg is a config with the assistant configured — a (fake) key, on, and a
// resolvable model. The key isn't real, but ValidateAgentModel/NewRunner only
// PARSE the spec (no live call), so a Runner still builds and capabilities
// reports it live. That's enough to exercise the runtime on/off swap.
func agentCfg() *config.Config {
c := localCfg()
c.Agent = config.AgentConfig{
Model: "ollama-cloud/glm-5.2:cloud",
OllamaCloudAPIKey: "test-key",
Enabled: true,
}
return c
}
func settingsVersion(t *testing.T, r *gin.Engine, cookie *http.Cookie) int64 {
t.Helper()
w := doJSON(t, r, http.MethodGet, "/api/v1/settings", nil, cookie)
if w.Code != http.StatusOK {
t.Fatalf("get settings: status %d, body %s", w.Code, w.Body.String())
}
st, _ := decodeMap(t, w.Body.Bytes())["settings"].(map[string]any)
return int64(st["version"].(float64))
}
// TestSettingsAdminOnly: the first registered user is admin and can read/write
// settings; a second user is not and gets 403 (not 404 — settings aren't a
// masked resource).
func TestSettingsAdminOnly(t *testing.T) {
r := authEngine(t, localCfg())
admin := registerAndCookie(t, r, "[email protected]") // first user → admin
member := registerAndCookie(t, r, "[email protected]")
if w := doJSON(t, r, http.MethodGet, "/api/v1/settings", nil, admin); w.Code != http.StatusOK {
t.Fatalf("admin GET settings: status %d, body %s", w.Code, w.Body.String())
}
if w := doJSON(t, r, http.MethodGet, "/api/v1/settings", nil, member); w.Code != http.StatusForbidden {
t.Errorf("member GET settings: status %d, want 403", w.Code)
}
if w := doJSON(t, r, http.MethodPatch, "/api/v1/settings",
map[string]any{"agentModel": "x", "version": 1}, member); w.Code != http.StatusForbidden {
t.Errorf("member PATCH settings: status %d, want 403", w.Code)
}
// Unauthenticated is 401, before the admin check.
if w := doJSON(t, r, http.MethodGet, "/api/v1/settings", nil, nil); w.Code != http.StatusUnauthorized {
t.Errorf("anonymous GET settings: status %d, want 401", w.Code)
}
}
// TestSettingsInheritFromEnv: an untouched instance reports the env model as
// effective, and an empty stored model keeps inheriting it.
func TestSettingsInheritFromEnv(t *testing.T) {
r := authEngine(t, agentCfg())
admin := registerAndCookie(t, r, "[email protected]")
w := doJSON(t, r, http.MethodGet, "/api/v1/settings", nil, admin)
if w.Code != http.StatusOK {
t.Fatalf("get: status %d, body %s", w.Code, w.Body.String())
}
body := decodeMap(t, w.Body.Bytes())
st := body["settings"].(map[string]any)
eff := body["effective"].(map[string]any)
if st["agentModel"] != "" {
t.Errorf("stored model = %v, want empty (inherit)", st["agentModel"])
}
if eff["model"] != "ollama-cloud/glm-5.2:cloud" {
t.Errorf("effective model = %v, want the env value", eff["model"])
}
if eff["hasApiKey"] != true || eff["agentLive"] != true {
t.Errorf("effective = %+v, want a key present and the agent live", eff)
}
}
// TestSettingsUpdateSwapsTheRunner is the core of #79: changing settings takes
// effect on the LIVE assistant, with no restart. Driven entirely through HTTP.
func TestSettingsUpdateSwapsTheRunner(t *testing.T) {
r := authEngine(t, agentCfg())
admin := registerAndCookie(t, r, "[email protected]")
capsAgent := func() bool {
w := doJSON(t, r, http.MethodGet, "/api/v1/capabilities", nil, admin)
return decodeMap(t, w.Body.Bytes())["agent"] == true
}
// Configured and on out of the box.
if !capsAgent() {
t.Fatal("assistant should be live at boot with a key + enabled")
}
// Turn it OFF via settings → capabilities flips immediately.
v := settingsVersion(t, r, admin)
w := doJSON(t, r, http.MethodPatch, "/api/v1/settings",
map[string]any{"agentModel": "", "agentEnabled": false, "version": v}, admin)
if w.Code != http.StatusOK {
t.Fatalf("disable: status %d, body %s", w.Code, w.Body.String())
}
if capsAgent() {
t.Error("assistant still live after being disabled — the runner wasn't swapped")
}
// Chat now refuses, at runtime, on a route that still exists.
gid := createGardenAPI(t, r, admin, "G")
if w := doJSON(t, r, http.MethodPost, "/api/v1/agent/chat",
map[string]any{"gardenId": gid, "message": "hi"}, admin); w.Code != http.StatusServiceUnavailable {
t.Errorf("chat while disabled: status %d, want 503", w.Code)
}
// Turn it back ON, with an explicit model, and confirm it's live again and the
// effective model reflects the change.
v = settingsVersion(t, r, admin)
w = doJSON(t, r, http.MethodPatch, "/api/v1/settings", map[string]any{
"agentModel": "ollama-cloud/kimi-k2.6:cloud", "agentEnabled": true, "version": v,
}, admin)
if w.Code != http.StatusOK {
t.Fatalf("re-enable: status %d, body %s", w.Code, w.Body.String())
}
if eff := decodeMap(t, w.Body.Bytes())["effective"].(map[string]any); eff["model"] != "ollama-cloud/kimi-k2.6:cloud" {
t.Errorf("effective model = %v after change, want the new one", eff["model"])
}
if !capsAgent() {
t.Error("assistant not live after being re-enabled")
}
}
// TestSettingsRejectsBadModel: a spec that won't resolve is a 400 at save time,
// not a broken assistant on the next turn.
func TestSettingsRejectsBadModel(t *testing.T) {
r := authEngine(t, agentCfg())
admin := registerAndCookie(t, r, "[email protected]")
v := settingsVersion(t, r, admin)
if w := doJSON(t, r, http.MethodPatch, "/api/v1/settings",
map[string]any{"agentModel": "nonesuch/model", "version": v}, admin); w.Code != http.StatusBadRequest {
t.Errorf("bad model: status %d, want 400", w.Code)
}
// agentEnabled must be a bool or null, not a string.
if w := doJSON(t, r, http.MethodPatch, "/api/v1/settings",
map[string]any{"agentModel": "", "agentEnabled": "yes", "version": v}, admin); w.Code != http.StatusBadRequest {
t.Errorf("string agentEnabled: status %d, want 400", w.Code)
}
}
// TestSettingsVersionConflict: a stale version 409s and carries the current row.
func TestSettingsVersionConflict(t *testing.T) {
r := authEngine(t, agentCfg())
admin := registerAndCookie(t, r, "[email protected]")
v := settingsVersion(t, r, admin)
// First write succeeds and bumps the version.
if w := doJSON(t, r, http.MethodPatch, "/api/v1/settings",
map[string]any{"agentModel": "", "agentEnabled": false, "version": v}, admin); w.Code != http.StatusOK {
t.Fatalf("first update: status %d, body %s", w.Code, w.Body.String())
}
// Reusing the old version conflicts.
w := doJSON(t, r, http.MethodPatch, "/api/v1/settings",
map[string]any{"agentModel": "", "agentEnabled": true, "version": v}, admin)
if w.Code != http.StatusConflict {
t.Fatalf("stale update: status %d, want 409", w.Code)
}
if cur, ok := decodeMap(t, w.Body.Bytes())["current"].(map[string]any); !ok || cur["version"].(float64) != float64(v+1) {
t.Errorf("409 body missing the current row at the bumped version: %s", w.Body.String())
}
}
// TestSettingsSwapUnderRace runs settings saves concurrently with chat requests,
// so `go test -race` proves the atomic swap of the live Runner is safe against
// in-flight readers. The whole point of the atomic.Pointer is this: without it,
// toggling the assistant while a request reads it is a data race.
func TestSettingsSwapUnderRace(t *testing.T) {
Review

🟠 Test goroutines may outlive test function causing race/panic under -race

error-handling · flagged by 1 model

  • internal/api/settings_test.go:180-208TestSettingsSwapUnderRace spawns 4 reader goroutines that loop until <-done, then closes done and returns immediately. The test does not wait for the goroutines to finish, so they may still be running (and calling doJSON, which uses testing.T) after the test function exits. This is a real race under go test -race that can panic or produce spurious race reports. Add a sync.WaitGroup and wait for the workers before returning. *Verif…

🪰 Gadfly · advisory

🟠 **Test goroutines may outlive test function causing race/panic under -race** _error-handling · flagged by 1 model_ - **`internal/api/settings_test.go:180-208`** — `TestSettingsSwapUnderRace` spawns 4 reader goroutines that loop until `<-done`, then closes `done` and returns immediately. The test does **not** wait for the goroutines to finish, so they may still be running (and calling `doJSON`, which uses `testing.T`) after the test function exits. This is a real race under `go test -race` that can panic or produce spurious race reports. Add a `sync.WaitGroup` and wait for the workers before returning. *Verif… <sub>🪰 Gadfly · advisory</sub>
r := authEngine(t, agentCfg())
admin := registerAndCookie(t, r, "[email protected]")
done := make(chan struct{})
// Readers hammer capabilities, whose h.agent.get() is the SAME atomic Load the
// chat handler does — so this races the pointer read against the writer's swap
// without ever invoking the model (a real Run would hit the network on a fake
// key). If get() is race-clean here it is race-clean in chat.
for i := 0; i < 4; i++ {
go func() {
for {
select {
case <-done:
return
default:
doJSON(t, r, http.MethodGet, "/api/v1/capabilities", nil, admin)
}
}
}()
}
// Writer: flip the assistant on and off, swapping the pointer each time.
for i := 0; i < 12; i++ {
v := settingsVersion(t, r, admin)
doJSON(t, r, http.MethodPatch, "/api/v1/settings",
map[string]any{"agentModel": "", "agentEnabled": i%2 == 0, "version": v}, admin)
}
close(done)
}
-7
View File
@@ -76,13 +76,6 @@ type AgentConfig struct {
Enabled bool
}
// Ready reports whether the assistant can actually be offered. Both the route
// registration and whatever advertises capabilities gate on this, so what's
// advertised always matches what's live.
func (a AgentConfig) Ready() bool {
return a.Enabled && a.OllamaCloudAPIKey != "" && a.Model != ""
}
// Enabled reports whether enough OIDC config is present to attempt discovery.
func (o OIDCConfig) Enabled() bool {
return o.Issuer != "" && o.ClientID != ""
+16
View File
@@ -242,6 +242,22 @@ const (
ConflictUnsupported = "unsupported"
)
// InstanceSettings is the single row of instance-wide, admin-editable
// configuration (#79). Secrets are deliberately absent — see migration 0010.
//
// Both agent fields express "inherit from the environment unless set":
// AgentModel == "" falls back to PANSY_AGENT_MODEL; AgentEnabled == nil inherits
// the env default. EffectiveAgent resolves them against the environment.
type InstanceSettings struct {
// AgentModel overrides PANSY_AGENT_MODEL when non-empty. Passed verbatim to
// majordomo.Parse, exactly like the env var it shadows.
AgentModel string `json:"agentModel"`
// AgentEnabled overrides PANSY_AGENT_ENABLED when non-nil. nil = inherit.
AgentEnabled *bool `json:"agentEnabled"`
Version int64 `json:"version"`
UpdatedAt string `json:"updatedAt"`
}
// User is a pansy account. It may have a local password, OIDC identity, or both.
type User struct {
ID int64 `json:"id"`
+110
View File
@@ -0,0 +1,110 @@
package service
import (
"context"
"strings"
"gitea.stevedudenhoeffer.com/steve/pansy/internal/agentmodel"
"gitea.stevedudenhoeffer.com/steve/pansy/internal/domain"
)
// Instance settings (#79): admin-editable, instance-wide configuration. The
// admin gate lives HERE, in the seam, not in the handler — the same rule every
// other permission follows. The API's requireAdmin middleware is a cheap early
// 403, not the authority.
// requireAdmin returns nil iff the actor is an admin, else ErrForbidden.
//
// ErrForbidden, not ErrNotFound: settings are not a resource whose existence is
// masked. A logged-in non-admin knows the instance has settings; they simply may
// not touch them. (Contrast objects/lots, where no-access masks existence.)
func (s *Service) requireAdmin(ctx context.Context, actorID int64) error {
u, err := s.store.GetUserByID(ctx, actorID)
if err != nil {
return err
}
if !u.IsAdmin {
return domain.ErrForbidden
Review

🟡 requireAdmin re-queries user table when IsAdmin already verified by middleware

performance · flagged by 1 model

  • internal/service/instance_settings.go:27-36requireAdmin queries GetUserByID to check IsAdmin on every settings request, but mustActor(c).IsAdmin was already resolved during session authentication and checked by the requireAdmin() middleware. This adds a DB roundtrip to an already-gated endpoint. The author calls this "redundant by design" (service seam authority), which is fair for correctness, but from a performance lens it's still avoidable overhead on an endpoint that does li…

🪰 Gadfly · advisory

🟡 **requireAdmin re-queries user table when IsAdmin already verified by middleware** _performance · flagged by 1 model_ - `internal/service/instance_settings.go:27-36` — `requireAdmin` queries `GetUserByID` to check `IsAdmin` on every settings request, but `mustActor(c).IsAdmin` was already resolved during session authentication and checked by the `requireAdmin()` middleware. This adds a DB roundtrip to an already-gated endpoint. The author calls this "redundant by design" (service seam authority), which is fair for correctness, but from a performance lens it's still avoidable overhead on an endpoint that does li… <sub>🪰 Gadfly · advisory</sub>
}
return nil
}
// GetInstanceSettings returns the instance settings for an admin.
func (s *Service) GetInstanceSettings(ctx context.Context, actorID int64) (*domain.InstanceSettings, error) {
if err := s.requireAdmin(ctx, actorID); err != nil {
return nil, err
}
return s.store.GetInstanceSettings(ctx)
}
// InstanceSettingsPatch is a full replacement of the editable fields plus the
// current version. Both agent fields carry their "inherit" sentinel: an empty
// AgentModel means fall back to env, a nil AgentEnabled means inherit.
type InstanceSettingsPatch struct {
AgentModel string
AgentEnabled *bool
Version int64
}
// UpdateInstanceSettings applies an admin's change, version-guarded. It returns
// (current row, ErrVersionConflict) on a stale version, like every mutable
// resource. The model spec is validated before it is stored, so a typo is a 400
// now rather than a broken assistant on the next turn.
//
// It does NOT rebuild the running agent — that is the API layer's job, because
// the live Runner lives there. The caller rebuilds on success.
func (s *Service) UpdateInstanceSettings(ctx context.Context, actorID int64, patch InstanceSettingsPatch) (*domain.InstanceSettings, error) {
if err := s.requireAdmin(ctx, actorID); err != nil {
return nil, err
}
model := strings.TrimSpace(patch.AgentModel)
// Validate a non-empty spec up front. An empty one is the "inherit env"
// sentinel and needs no check — the env value was validated at boot.
if model != "" {
if err := agentmodel.Validate(s.cfg.Agent.OllamaCloudAPIKey, model); err != nil {
Review

🟠 Swallowed validation error leaves admin with no diagnostic on bad model spec

error-handling · flagged by 1 model

  • internal/service/instance_settings.go:64agentmodel.Validate returns a specific error (e.g. empty spec, unknown provider, malformed failover chain), but UpdateInstanceSettings swallows it and returns a generic domain.ErrInvalidInput. An admin POSTing a bad model gets a 400 with code INVALID_INPUT and message "invalid input", with no hint that it's the model spec that failed. The underlying err should at least be logged so an operator can diagnose it, or the API should surfa…

🪰 Gadfly · advisory

🟠 **Swallowed validation error leaves admin with no diagnostic on bad model spec** _error-handling · flagged by 1 model_ - **`internal/service/instance_settings.go:64`** — `agentmodel.Validate` returns a specific error (e.g. empty spec, unknown provider, malformed failover chain), but `UpdateInstanceSettings` swallows it and returns a generic `domain.ErrInvalidInput`. An admin POSTing a bad model gets a 400 with code `INVALID_INPUT` and message `"invalid input"`, with no hint that it's the model spec that failed. The underlying `err` should at least be logged so an operator can diagnose it, or the API should surfa… <sub>🪰 Gadfly · advisory</sub>
return nil, domain.ErrInvalidInput
}
}
return s.store.UpdateInstanceSettings(ctx, &domain.InstanceSettings{
AgentModel: model,
AgentEnabled: patch.AgentEnabled,
Version: patch.Version,
})
}
// EffectiveAgent resolves the agent configuration actually in force: DB settings
// override the environment, and the API key always comes from the environment.
//
// This is internal plumbing (the API layer calls it to build the Runner), NOT an
// admin-gated operation — resolving what's configured is not the same as editing
// it, and the agent bootstrap must work before any user is even authenticated.
type EffectiveAgent struct {
Review

🟠 EffectiveAgent type and method share the same name in one package

maintainability · flagged by 1 model

  • internal/service/instance_settings.go:81 and :93EffectiveAgent is both a struct type and a *Service method in the same package. This hurts godoc readability and IDE navigation; searching for the type jumps to the method and vice-versa. Rename the method to something distinct like ResolveEffectiveAgent or CurrentAgentConfig.

🪰 Gadfly · advisory

🟠 **EffectiveAgent type and method share the same name in one package** _maintainability · flagged by 1 model_ - **`internal/service/instance_settings.go:81`** and `:93` — `EffectiveAgent` is both a struct type and a `*Service` method in the same package. This hurts godoc readability and IDE navigation; searching for the type jumps to the method and vice-versa. Rename the method to something distinct like `ResolveEffectiveAgent` or `CurrentAgentConfig`. <sub>🪰 Gadfly · advisory</sub>
Model string
Enabled bool
APIKey string
}
// Ready mirrors config.AgentConfig.Ready: enabled, with a key and a model.
Review

🟡 Stale doc comment references config.AgentConfig.Ready(), which this PR deletes

maintainability · flagged by 2 models

  1. internal/service/instance_settings.go:87 — comment references config.AgentConfig.Ready, which no longer exists (grep confirms zero definitions). The EffectiveAgent.Ready method exists but the comment points at the deleted one.

🪰 Gadfly · advisory

🟡 **Stale doc comment references config.AgentConfig.Ready(), which this PR deletes** _maintainability · flagged by 2 models_ 1. `internal/service/instance_settings.go:87` — comment references `config.AgentConfig.Ready`, which no longer exists (grep confirms zero definitions). The `EffectiveAgent.Ready` method exists but the comment points at the deleted one. <sub>🪰 Gadfly · advisory</sub>
func (e EffectiveAgent) Ready() bool {
return e.Enabled && e.APIKey != "" && e.Model != ""
}
// EffectiveAgent reads the settings row and layers it over the environment.
func (s *Service) EffectiveAgent(ctx context.Context) (EffectiveAgent, error) {
st, err := s.store.GetInstanceSettings(ctx)
if err != nil {
return EffectiveAgent{}, err
}
eff := EffectiveAgent{
Model: s.cfg.Agent.Model,
Enabled: s.cfg.Agent.Enabled,
APIKey: s.cfg.Agent.OllamaCloudAPIKey,
}
if st.AgentModel != "" {
eff.Model = st.AgentModel
}
if st.AgentEnabled != nil {
eff.Enabled = *st.AgentEnabled
}
return eff, nil
}
+117
View File
@@ -0,0 +1,117 @@
package service
import (
"context"
"errors"
"testing"
"gitea.stevedudenhoeffer.com/steve/pansy/internal/config"
"gitea.stevedudenhoeffer.com/steve/pansy/internal/domain"
)
// settingsTestService builds a service whose env config carries the given agent
// model/enabled/key, so EffectiveAgent's env fallback can be exercised.
func settingsTestService(t *testing.T, envModel string, envEnabled bool, key string) (*Service, int64) {
t.Helper()
cfg := openConfig()
cfg.Agent = config.AgentConfig{Model: envModel, Enabled: envEnabled, OllamaCloudAPIKey: key}
s := newTestService(t, cfg)
admin := seedUser(t, s, "[email protected]") // first user is admin
return s, admin
}
// TestRequireAdmin: the first user is admin; a second is not and gets
// ErrForbidden (not ErrNotFound — settings existence isn't masked).
func TestRequireAdmin(t *testing.T) {
s, admin := settingsTestService(t, "ollama-cloud/x", true, "k")
member := seedUser(t, s, "[email protected]")
if err := s.requireAdmin(context.Background(), admin); err != nil {
t.Errorf("admin rejected: %v", err)
}
if err := s.requireAdmin(context.Background(), member); !errors.Is(err, domain.ErrForbidden) {
t.Errorf("member requireAdmin = %v, want ErrForbidden", err)
}
}
// TestEffectiveAgentLayering: DB settings override env; the API key always comes
// from env; the "inherit" sentinels fall back.
func TestEffectiveAgentLayering(t *testing.T) {
ctx := context.Background()
s, admin := settingsTestService(t, "ollama-cloud/env-model", true, "envkey")
// Untouched: everything inherits env.
eff, err := s.EffectiveAgent(ctx)
if err != nil {
t.Fatalf("effective: %v", err)
}
if eff.Model != "ollama-cloud/env-model" || !eff.Enabled || eff.APIKey != "envkey" {
t.Errorf("inherited effective = %+v, want the env values", eff)
}
// Override the model only; enabled still inherits env (true).
cur, _ := s.GetInstanceSettings(ctx, admin)
if _, err := s.UpdateInstanceSettings(ctx, admin, InstanceSettingsPatch{
AgentModel: "ollama-cloud/glm-5.2:cloud", Version: cur.Version,
}); err != nil {
t.Fatalf("update model: %v", err)
}
eff, _ = s.EffectiveAgent(ctx)
if eff.Model != "ollama-cloud/glm-5.2:cloud" {
t.Errorf("model = %q, want the DB override", eff.Model)
}
if !eff.Enabled {
t.Error("enabled should still inherit env (true) when unset")
}
// Now override enabled to false explicitly.
cur, _ = s.GetInstanceSettings(ctx, admin)
no := false
if _, err := s.UpdateInstanceSettings(ctx, admin, InstanceSettingsPatch{
AgentModel: "ollama-cloud/glm-5.2:cloud", AgentEnabled: &no, Version: cur.Version,
}); err != nil {
t.Fatalf("update enabled: %v", err)
}
eff, _ = s.EffectiveAgent(ctx)
if eff.Enabled {
t.Error("enabled should be the explicit false override now")
}
if eff.Ready() {
t.Error("Ready() should be false when disabled")
}
}
// TestUpdateInstanceSettingsRejectsBadModel: a spec that won't resolve is
// ErrInvalidInput, before it is stored.
func TestUpdateInstanceSettingsRejectsBadModel(t *testing.T) {
ctx := context.Background()
s, admin := settingsTestService(t, "ollama-cloud/x", true, "k")
cur, _ := s.GetInstanceSettings(ctx, admin)
if _, err := s.UpdateInstanceSettings(ctx, admin, InstanceSettingsPatch{
AgentModel: "nonesuch/model", Version: cur.Version,
}); !errors.Is(err, domain.ErrInvalidInput) {
t.Errorf("bad model = %v, want ErrInvalidInput", err)
}
// The rejected write didn't touch the row.
after, _ := s.GetInstanceSettings(ctx, admin)
if after.Version != cur.Version || after.AgentModel != "" {
t.Errorf("a rejected update changed the row: %+v", after)
}
}
// TestInstanceSettingsAdminGate: the read/write operations are admin-gated at the
// service seam, not just in the handler.
func TestInstanceSettingsAdminGate(t *testing.T) {
ctx := context.Background()
s, _ := settingsTestService(t, "ollama-cloud/x", true, "k")
member := seedUser(t, s, "[email protected]")
if _, err := s.GetInstanceSettings(ctx, member); !errors.Is(err, domain.ErrForbidden) {
t.Errorf("member GetInstanceSettings = %v, want ErrForbidden", err)
}
if _, err := s.UpdateInstanceSettings(ctx, member, InstanceSettingsPatch{Version: 1}); !errors.Is(err, domain.ErrForbidden) {
t.Errorf("member UpdateInstanceSettings = %v, want ErrForbidden", err)
}
}
+81
View File
@@ -0,0 +1,81 @@
package store
import (
"context"
"database/sql"
"errors"
"fmt"
"gitea.stevedudenhoeffer.com/steve/pansy/internal/domain"
)
// The instance_settings row is seeded by migration 0010 and there is exactly one
// (CHECK id = 1), so reads never branch on existence and writes never insert.
const instanceSettingsColumns = `agent_model, agent_enabled, version, updated_at`
// scanInstanceSettings reads the single settings row. agent_enabled is a nullable
// INTEGER (NULL = inherit env), so it is scanned through sql.NullInt64.
func scanInstanceSettings(s scanner) (*domain.InstanceSettings, error) {
var (
out domain.InstanceSettings
enabled sql.NullInt64
)
if err := s.Scan(&out.AgentModel, &enabled, &out.Version, &out.UpdatedAt); err != nil {
return nil, err
}
if enabled.Valid {
b := enabled.Int64 != 0
out.AgentEnabled = &b
}
return &out, nil
}
// GetInstanceSettings returns the single settings row.
func (d *DB) GetInstanceSettings(ctx context.Context) (*domain.InstanceSettings, error) {
s, err := scanInstanceSettings(d.sql.QueryRowContext(ctx,
`SELECT `+instanceSettingsColumns+` FROM instance_settings WHERE id = 1`))
if errors.Is(err, sql.ErrNoRows) {
// The migration seeds this row, so its absence is a broken database, not a
// normal "not found" the caller should paper over.
return nil, fmt.Errorf("store: instance_settings row missing (migration 0010 not applied?)")
}
if err != nil {
return nil, fmt.Errorf("store: get instance settings: %w", err)
}
return s, nil
}
// UpdateInstanceSettings applies a version-guarded update to the single row,
// following the same optimistic-concurrency contract as every mutable resource:
// the updated row on success, (current row, ErrVersionConflict) on a version
// mismatch. There is no ErrNotFound path — the row always exists.
//
// agentEnabled is nil to store SQL NULL (inherit env), or a pointer to store an
// explicit 0/1.
func (d *DB) UpdateInstanceSettings(ctx context.Context, s *domain.InstanceSettings) (*domain.InstanceSettings, error) {
var enabled any
if s.AgentEnabled != nil {
enabled = boolToInt(*s.AgentEnabled)
}
updated, err := scanInstanceSettings(d.sql.QueryRowContext(ctx,
`UPDATE instance_settings
SET agent_model = ?, agent_enabled = ?,
version = version + 1,
updated_at = strftime('%Y-%m-%dT%H:%M:%SZ', 'now')
WHERE id = 1 AND version = ?
RETURNING `+instanceSettingsColumns,
s.AgentModel, enabled, s.Version,
))
if errors.Is(err, sql.ErrNoRows) {
current, gerr := d.GetInstanceSettings(ctx)
if gerr != nil {
return nil, gerr
}
return current, domain.ErrVersionConflict
}
if err != nil {
return nil, fmt.Errorf("store: update instance settings: %w", err)
}
return updated, nil
}
@@ -0,0 +1,35 @@
-- Instance settings (#79): the first configuration that lives in the database
-- rather than the environment.
--
-- Until now every preference hung off a garden or object row; this is pansy's
-- first INSTANCE-level state. A single-row table (CHECK id = 1) is the least
-- surprising shape for "there is exactly one of these" — a key/value table would
-- invite typo'd keys and lose the column types.
--
-- Only NON-SECRET agent settings live here. OLLAMA_CLOUD_API_KEY stays in the
-- environment on purpose: a copy in SQLite would land in every backup and in the
-- blast radius of the undo history. An admin can change WHICH model runs, not
-- WHOSE account pays for it.
--
-- Both agent columns are "inherit from env unless set":
-- * agent_model = '' means fall back to PANSY_AGENT_MODEL, then the built-in
-- default. So an instance that never opens Settings behaves exactly as it
-- did before this migration, and the documented env var keeps working.
-- * agent_enabled is NULLABLE: NULL means inherit PANSY_AGENT_ENABLED's
-- behaviour (on when a key is present), 0/1 is an explicit override. A plain
-- boolean couldn't tell "admin hasn't touched this" from "admin turned it
-- off", and those must deploy differently.
--
-- version drives the same optimistic-concurrency 409 every other mutable row
-- uses, so two admins editing at once conflict rather than clobber.
CREATE TABLE instance_settings (
id INTEGER PRIMARY KEY CHECK (id = 1),
agent_model TEXT NOT NULL DEFAULT '',
agent_enabled INTEGER CHECK (agent_enabled IN (0, 1)),
version INTEGER NOT NULL DEFAULT 1,
updated_at TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%SZ', 'now'))
);
-- Seed the single row so every read is a plain SELECT with no "does it exist
-- yet" branch. Inherits everything from the environment out of the box.
INSERT INTO instance_settings (id, agent_model, agent_enabled) VALUES (1, '', NULL);
+13
View File
@@ -53,6 +53,19 @@ export function AppShell() {
{l.label}
</Link>
))}
{/* Settings is admin-only, matching the server's requireAdmin gate.
A non-admin who typed /settings still gets a 403 from the API — the
hidden link is convenience, not the security boundary. */}
{user?.isAdmin && (
<Link
to="/settings"
className={navLinkBase}
activeProps={{ className: navLinkActive }}
inactiveProps={{ className: navLinkInactive }}
>
Settings
</Link>
)}
</div>
{user ? (
+20 -7
View File
@@ -13,13 +13,21 @@ import { historyKey } from './history'
const capabilitiesSchema = z.object({ agent: z.boolean() })
/** Whether this instance has the assistant configured. Without it the panel
* isn't rendered at all — a dead button is worse than no button. */
export const capabilitiesKey = ['capabilities'] as const
/** Whether the assistant is live RIGHT NOW. Without it the panel isn't rendered
* at all — a dead button is worse than no button.
*
* Not `staleTime: Infinity` any more: an admin can turn the assistant on or off
* in Settings (#79), so this must be able to change under a running page. The
* settings save invalidates this key directly; the finite staleTime just means
* another admin's change is picked up on the next focus/remount rather than
* never. */
export function useCapabilities() {
return useQuery({
queryKey: ['capabilities'] as const,
queryKey: capabilitiesKey,
queryFn: async () => capabilitiesSchema.parse(await api.get('/capabilities')),
staleTime: Infinity, // server config; it doesn't change under a running page
staleTime: 60_000,
})
}
@@ -153,10 +161,15 @@ export async function streamChat(
return
}
if (!res.ok || !res.body) {
// 503 is the assistant being turned off at runtime (#79) — the route exists,
// there's just no model behind it. Distinct from a 404, which would mean the
// whole endpoint is absent.
handlers.onError(
res.status === 404
? "This instance doesn't have the assistant configured."
: 'The assistant is not available right now.',
res.status === 503
? "The assistant isn't enabled on this instance."
: res.status === 404
? "This instance doesn't have the assistant configured."
: 'The assistant is not available right now.',
)
return
}
+79
View File
@@ -0,0 +1,79 @@
// Instance settings data layer (#79): admin-only, instance-wide configuration.
//
// The GET/PATCH return both the stored settings and a read-only "effective" view
// — what's actually in force after layering the DB over the environment — so the
// form can say "inheriting ollama-cloud/glm-5.2:cloud from the environment" and
// whether the API key is present, without the key ever crossing the wire.
import { queryOptions, useMutation, useQuery, useQueryClient } from '@tanstack/react-query'
import { z } from 'zod'
import { ApiError, api } from './api'
import { capabilitiesKey } from './agent'
export const instanceSettingsSchema = z.object({
// '' means "inherit the PANSY_AGENT_MODEL env var".
agentModel: z.string(),
// null means "inherit PANSY_AGENT_ENABLED"; true/false is an explicit override.
agentEnabled: z.boolean().nullable(),
version: z.number(),
updatedAt: z.string(),
})
export type InstanceSettings = z.infer<typeof instanceSettingsSchema>
export const effectiveAgentSchema = z.object({
model: z.string(),
enabled: z.boolean(),
hasApiKey: z.boolean(),
agentLive: z.boolean(),
})
export type EffectiveAgent = z.infer<typeof effectiveAgentSchema>
export const settingsResponseSchema = z.object({
settings: instanceSettingsSchema,
effective: effectiveAgentSchema,
})
export type SettingsResponse = z.infer<typeof settingsResponseSchema>
export const settingsKey = ['settings'] as const
export const settingsQueryOptions = queryOptions({
queryKey: settingsKey,
queryFn: async (): Promise<SettingsResponse> =>
settingsResponseSchema.parse(await api.get('/settings')),
})
export function useSettings() {
return useQuery(settingsQueryOptions)
}
export interface SettingsUpdate {
agentModel: string
agentEnabled: boolean | null
version: number
}
export function useUpdateSettings() {
const qc = useQueryClient()
return useMutation({
mutationFn: async (input: SettingsUpdate): Promise<SettingsResponse> =>
settingsResponseSchema.parse(await api.patch('/settings', input)),
onSuccess: (res) => {
qc.setQueryData(settingsKey, res)
// The save may have turned the assistant on or off; the editor keys its
// chat tab off /capabilities, so make it re-read rather than trust its
// cached answer.
qc.invalidateQueries({ queryKey: capabilitiesKey })
},
})
}
/** If err is a 409 version conflict, return the fresh settings it carries so a
* form can rebase; otherwise null. */
export function conflictSettings(err: unknown): InstanceSettings | null {
if (err instanceof ApiError && err.isConflict && err.body && typeof err.body === 'object') {
const current = (err.body as { current?: unknown }).current
const parsed = instanceSettingsSchema.safeParse(current)
if (parsed.success) return parsed.data
}
return null
}
+164
View File
@@ -0,0 +1,164 @@
import { useEffect, useState } from 'react'
import { Alert } from '@/components/ui/Alert'
import { Button } from '@/components/ui/Button'
import { Select } from '@/components/ui/Select'
import { TextField } from '@/components/ui/TextField'
import { toast } from '@/components/ui/toast'
import { errorMessage } from '@/lib/api'
import {
conflictSettings,
useSettings,
useUpdateSettings,
type EffectiveAgent,
type InstanceSettings,
} from '@/lib/settings'
import { usePageTitle } from '@/lib/usePageTitle'
// agentEnabled is a tri-state on the wire (null = inherit env, true, false); the
// form models it as three named choices so "inherit" is a deliberate pick, not
// an empty control.
type EnabledChoice = 'inherit' | 'on' | 'off'
const toChoice = (v: boolean | null): EnabledChoice => (v === null ? 'inherit' : v ? 'on' : 'off')
const fromChoice = (c: EnabledChoice): boolean | null => (c === 'inherit' ? null : c === 'on')
Review

🟠 fromChoice silently falls through 'off' in cryptic ternary

maintainability · flagged by 1 model

  • web/src/pages/SettingsPage.tsx:22fromChoice relies on a cryptic ternary (c === 'inherit' ? null : c === 'on') where 'off' silently falls through to false because 'off' !== 'on'. It happens to be correct today, but if EnabledChoice ever gains a value the compiler won't catch the missing branch and the function will silently misbehave. Use an explicit switch or if/else chain so every choice is handled deliberately.

🪰 Gadfly · advisory

🟠 **fromChoice silently falls through 'off' in cryptic ternary** _maintainability · flagged by 1 model_ - **`web/src/pages/SettingsPage.tsx:22`** — `fromChoice` relies on a cryptic ternary (`c === 'inherit' ? null : c === 'on'`) where `'off'` silently falls through to `false` because `'off' !== 'on'`. It happens to be correct today, but if `EnabledChoice` ever gains a value the compiler won't catch the missing branch and the function will silently misbehave. Use an explicit `switch` or `if/else` chain so every choice is handled deliberately. <sub>🪰 Gadfly · advisory</sub>
/** Admin-only instance settings (#79). The one that matters today is the agent
* model; the API key stays in the environment and is only ever reported as
* present/absent, never shown or edited. */
export function SettingsPage() {
usePageTitle('Settings')
const settings = useSettings()
const update = useUpdateSettings()
if (settings.isPending) {
return <p className="text-sm text-muted">Loading settings</p>
}
if (settings.isError) {
return <Alert>{errorMessage(settings.error, "Couldn't load settings.")}</Alert>
}
return <SettingsForm data={settings.data} update={update} />
}
function SettingsForm({
data,
update,
}: {
data: { settings: InstanceSettings; effective: EffectiveAgent }
update: ReturnType<typeof useUpdateSettings>
}) {
const [model, setModel] = useState(data.settings.agentModel)
const [enabled, setEnabled] = useState<EnabledChoice>(toChoice(data.settings.agentEnabled))
const [version, setVersion] = useState(data.settings.version)
const [error, setError] = useState<string | null>(null)
// Rebase the form when the query cache updates (e.g. a successful save writes
// the new row back), so the version we submit is never stale.
useEffect(() => {
setVersion(data.settings.version)
}, [data.settings.version])
const eff = data.effective
const save = () => {
setError(null)
update.mutate(
{ agentModel: model.trim(), agentEnabled: fromChoice(enabled), version },
{
onSuccess: () => toast.info('Settings saved.'),
onError: (err) => {
const current = conflictSettings(err)
if (current) {
// Someone else saved first. Adopt their row so the next attempt is
// clean, and say so rather than silently discarding this edit.
setModel(current.agentModel)
setEnabled(toChoice(current.agentEnabled))
setVersion(current.version)
setError('Someone else changed these settings just now — reloaded their version. Re-apply your change if you still want it.')
return
}
setError(errorMessage(err, "Couldn't save settings."))
},
},
)
}
return (
<div className="flex max-w-2xl flex-col gap-6">
<div>
<h1 className="text-xl font-semibold text-fg">Settings</h1>
<p className="mt-1 text-sm text-muted">
Instance-wide, admin-only. Changes take effect immediately no restart.
</p>
</div>
<section className="flex flex-col gap-4 rounded-lg border border-border p-4">
<div>
<h2 className="text-sm font-semibold text-fg">Garden assistant</h2>
<p className="mt-1 text-xs text-muted">
The model runs against your Ollama Cloud key, which is set in the environment and never
shown here.
</p>
</div>
<AgentStatus eff={eff} />
<TextField
label="Model"
name="agentModel"
placeholder={eff.model || 'ollama-cloud/glm-5.2:cloud'}
value={model}
onChange={(e) => setModel(e.target.value)}
hint={
model.trim() === ''
? `Empty — inheriting ${eff.model || 'the built-in default'} from the environment.`
: 'A majordomo model spec. A comma-separated list is a failover chain.'
}
/>
<Select
label="Enabled"
name="agentEnabled"
value={enabled}
onChange={(e) => setEnabled(e.target.value as EnabledChoice)}
options={[
{ value: 'inherit', label: 'Inherit from environment' },
{ value: 'on', label: 'On' },
{ value: 'off', label: 'Off' },
]}
/>
</section>
{error && <Alert>{error}</Alert>}
<div className="flex justify-end">
<Button onClick={save} disabled={update.isPending}>
{update.isPending ? 'Saving…' : 'Save changes'}
</Button>
</div>
</div>
)
}
// AgentStatus surfaces the gap the capabilities endpoint exists for: enabled +
// a key does not guarantee the assistant is actually running (an unresolvable
// model leaves it down), and that's precisely what an admin needs to see.
function AgentStatus({ eff }: { eff: EffectiveAgent }) {
const [tone, text] = eff.agentLive
? (['ok', `Live on ${eff.model}`] as const)
: !eff.hasApiKey
? (['warn', 'No API key set in the environment — the assistant is off.'] as const)
: !eff.enabled
? (['warn', 'Turned off.'] as const)
: (['warn', `Configured but not running — check the model (${eff.model}).`] as const)
return (
<div className="flex items-center gap-2 text-sm">
<span
className={
'inline-block h-2 w-2 rounded-full ' + (tone === 'ok' ? 'bg-emerald-500' : 'bg-amber-500')
}
aria-hidden
/>
<span className={tone === 'ok' ? 'text-fg' : 'text-muted'}>{text}</span>
</div>
)
}
+23
View File
@@ -14,6 +14,7 @@ import { GardensPage } from '@/pages/GardensPage'
import { GardenEditorPage } from '@/pages/GardenEditorPage'
import { PublicGardenPage } from '@/pages/PublicGardenPage'
import { PlantsPage } from '@/pages/PlantsPage'
import { SettingsPage } from '@/pages/SettingsPage'
import { meQueryOptions } from '@/lib/auth'
import { queryClient } from '@/lib/queryClient'
import { safeRedirectPath } from '@/lib/redirect'
@@ -48,6 +49,20 @@ async function requireGuest(context: RouterContext, redirectTo: string) {
}
}
// requireAdmin: authenticated AND admin. A non-admin who navigates to /settings
// is sent to /gardens rather than shown a page whose API calls would 403. This
// is convenience routing, not the security boundary — the server's requireAdmin
// is authoritative.
async function requireAdmin(context: RouterContext, path: string) {
const me = await context.queryClient.ensureQueryData(meQueryOptions)
if (!me) {
throw redirect({ to: '/login', search: { redirect: path } })
}
if (!me.isAdmin) {
throw redirect({ to: '/gardens' })
}
}
const indexRoute = createRoute({
getParentRoute: () => rootRoute,
path: '/',
@@ -109,6 +124,13 @@ const plantsRoute = createRoute({
component: PlantsPage,
})
const settingsRoute = createRoute({
getParentRoute: () => rootRoute,
path: 'settings',
beforeLoad: ({ context, location }) => requireAdmin(context, location.href),
component: SettingsPage,
})
// Public read-only garden by share token. Deliberately has NO beforeLoad auth
// guard, so a logged-out visitor viewing a shared link is never redirected to
// /login or OIDC.
@@ -125,6 +147,7 @@ const routeTree = rootRoute.addChildren([
gardensRoute,
gardenEditorRoute,
plantsRoute,
settingsRoute,
publicGardenRoute,
])