Admin-gated Settings: runtime model selection, enforce is_admin (#79)
Moves the agent model out of env-only config into an admin-editable Settings
section, and enforces is_admin for the first time — it has been in the schema
since migration 0001, plumbed to the client, and checked nowhere.
Backend:
- Migration 0010: instance_settings, a single-row (CHECK id=1) table — pansy's
first instance-level state. Holds agent_model ('' = inherit env) and
agent_enabled (NULL = inherit env), version-guarded like every mutable row.
SECRETS STAY IN ENV: OLLAMA_CLOUD_API_KEY is never stored here.
- requireAdmin at the service seam (authoritative) plus a cheap middleware
early-403. Non-admin gets 403, not 404 — settings existence isn't masked.
- EffectiveAgent resolves DB-over-env (model, enabled); key always from env.
- The live Runner is hot-swapped, not built once. agentHolder holds it behind
an atomic.Pointer; the chat routes are now registered UNCONDITIONALLY and
nil-check agent.get(), so a settings change turns the assistant on/off/onto a
new model with no restart and no race against in-flight readers. /capabilities
reads the pointer, so it reports what's live, not what booted.
- internal/agentmodel is a new leaf package holding the one place that knows how
to turn a spec into a model. Both agent (to run) and service (to validate a
spec before storing it) import it; it can't live in agent, which imports
service. Settings PATCH validates the spec via Parse, so a typo is a 400 now
rather than a broken assistant on the next turn.
Frontend:
- /settings route (admin guard), a Settings page (model field, tri-state
enabled, live status), nav link shown only to admins.
- useCapabilities drops staleTime:Infinity — the assistant can now change under
a running page — and the settings save invalidates it.
Contract change: chat routes always exist, so "assistant off" is a runtime 503
+ capabilities:false, not a missing route. Updated the test that asserted the
old shape.
Verified live against the built binary: disable flips capabilities to false and
logs it; re-enable with a new model swaps it back; a bad spec is rejected 400;
the setting persists across a restart. Swap is race-clean under `go test -race`.
Docs: README (precedence + key-stays-in-env), DESIGN (decision + routes),
CLAUDE (don't re-add conditional route registration; key never in the DB).
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01H3zbym8Doka2d7D48maSgZ
This commit is contained in:
+13
-23
@@ -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,21 @@ 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 == "" || strings.TrimSpace(modelSpec) == "" {
|
||||
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)
|
||||
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
|
||||
}
|
||||
|
||||
@@ -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")
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user