Add settings page + settings file design spec
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,203 @@
|
|||||||
|
# Settings file + settings page — design
|
||||||
|
|
||||||
|
**Date:** 2026-06-01
|
||||||
|
**Status:** Approved (brainstorming), pending implementation plan
|
||||||
|
|
||||||
|
## Problem
|
||||||
|
|
||||||
|
The three configurable inputs — AI backend, Marvin tasks command, and knowledge
|
||||||
|
profile path — are read once from `ANTIDRIFT_*` environment variables at daemon
|
||||||
|
startup (`cmd/antidriftd/main.go`). Changing any of them means editing a launch
|
||||||
|
command and restarting. The user wants to edit them at runtime from the web UI,
|
||||||
|
backed by a persisted settings file, and to pick the knowledge profile with a
|
||||||
|
proper file picker rather than typing an absolute path.
|
||||||
|
|
||||||
|
A partial precedent already exists: `POST /knowledge/path` → `SetKnowledgePath`
|
||||||
|
re-points the profile live while planning, but the choice is session-only and
|
||||||
|
not persisted.
|
||||||
|
|
||||||
|
## Decisions (locked during brainstorming)
|
||||||
|
|
||||||
|
1. **Live apply** — saving a setting re-wires the running daemon immediately; no
|
||||||
|
restart. (Knowledge path already works this way.)
|
||||||
|
2. **Server-side browse endpoint** for the file picker — a browser `<input
|
||||||
|
type=file>` yields sandboxed file *contents*, not a server-side *path* the
|
||||||
|
daemon can re-read, so we expose a directory-listing endpoint and render our
|
||||||
|
own navigable picker.
|
||||||
|
3. **Env seeds first run, then file wins** — on first start with no settings
|
||||||
|
file, seed it from current env vars (or built-in defaults); thereafter the
|
||||||
|
settings file is the sole source of truth and env vars are ignored.
|
||||||
|
4. **Scope: just the three settings** — AI backend, Marvin command, knowledge
|
||||||
|
path. No port / default timebox / default enforcement in this version.
|
||||||
|
5. **Gear toggle on the main card** — a header gear icon toggles a settings
|
||||||
|
overlay in the existing single-page, SSE-driven UI; no new route.
|
||||||
|
|
||||||
|
## Architecture
|
||||||
|
|
||||||
|
### Key layering move: inject an "applier" closure
|
||||||
|
|
||||||
|
`web` must NOT import `ai` / `tasks` / `knowledge`. Re-wiring adapters is a
|
||||||
|
composition concern, not session state, so it does not belong on the controller
|
||||||
|
either. Instead `main.go` (the composition root) builds:
|
||||||
|
|
||||||
|
```go
|
||||||
|
applyFn := func(s settings.Settings) error { /* construct adapters, call ctrl setters */ }
|
||||||
|
```
|
||||||
|
|
||||||
|
and injects it into the server, exactly as `SetCoach` / `SetTasks` already inject
|
||||||
|
behavior. The HTTP handler shrinks to: validate → save file → call applier →
|
||||||
|
broadcast. `web` depends only on the `settings.Settings` type and the injected
|
||||||
|
closure.
|
||||||
|
|
||||||
|
Dependency direction: `web → settings` (type only) and `web → applyFn` (injected).
|
||||||
|
`main → {settings, ai, tasks, knowledge, session, web}`. No cycles.
|
||||||
|
|
||||||
|
### New package: `internal/settings`
|
||||||
|
|
||||||
|
A leaf type package, importing nothing else in the app.
|
||||||
|
|
||||||
|
```go
|
||||||
|
type Settings struct {
|
||||||
|
AIBackend string `json:"ai_backend"`
|
||||||
|
MarvinCmd string `json:"marvin_cmd"`
|
||||||
|
KnowledgePath string `json:"knowledge_path"`
|
||||||
|
}
|
||||||
|
|
||||||
|
func DefaultPath() (string, error) // ~/.antidrift/settings.json
|
||||||
|
func Load(path string) (Settings, error)
|
||||||
|
func Save(path string, s Settings) error // atomic temp+rename, mirrors store
|
||||||
|
func SeedFromEnv() Settings // reads ANTIDRIFT_AI_BACKEND/_MARVIN_CMD/_KNOWLEDGE_FILE
|
||||||
|
```
|
||||||
|
|
||||||
|
Settings file (`~/.antidrift/settings.json`, next to `state.json`):
|
||||||
|
|
||||||
|
```json
|
||||||
|
{
|
||||||
|
"ai_backend": "claude",
|
||||||
|
"marvin_cmd": "uv run --project /home/felixm/dev/ampy am --json --config /home/felixm/dev/ampy/config.json",
|
||||||
|
"knowledge_path": "/home/felixm/.antidrift/knowledge.md"
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
### Startup wiring (`main.go`)
|
||||||
|
|
||||||
|
Replace the three inline `os.Getenv(...)` wirings with:
|
||||||
|
|
||||||
|
1. Resolve `settings.DefaultPath()`.
|
||||||
|
2. If the file is absent: `s := settings.SeedFromEnv()` (filling built-in
|
||||||
|
defaults where env is unset), then `settings.Save(path, s)`.
|
||||||
|
If present: `s, _ := settings.Load(path)` and env is ignored.
|
||||||
|
3. `applyFn(s)` performs the wiring main does today:
|
||||||
|
- `b, err := ai.NewBackend(s.AIBackend)`; on error log + skip AI (as today);
|
||||||
|
else `svc := ai.NewService(b)` and call `SetCoach` / `SetDriftJudge` /
|
||||||
|
`SetNudge` / `SetReviewer`.
|
||||||
|
- `ctrl.SetTasks(tasks.NewMarvin(s.MarvinCmd))`.
|
||||||
|
- Knowledge unified: base source `ctrl.SetKnowledge(knowledge.NewFileSource(""))`
|
||||||
|
once, then `ctrl.SetKnowledgePath(s.KnowledgePath)` to drive the actual
|
||||||
|
path. Startup and live-edit then take the identical code path.
|
||||||
|
4. Hand `applyFn`, the loaded `s`, and the settings file path to the server.
|
||||||
|
|
||||||
|
### HTTP endpoints (`web`)
|
||||||
|
|
||||||
|
Added in a new `internal/web/settings_handlers.go` to keep `web.go` focused. The
|
||||||
|
server gains mutex-guarded fields: current `settings.Settings`, the settings file
|
||||||
|
path, and the injected `applyFn`.
|
||||||
|
|
||||||
|
- `GET /settings` → current settings as JSON.
|
||||||
|
- `POST /settings` → body is the full settings object. The handler calls
|
||||||
|
`applyFn(s)` to validate-and-apply atomically: `applyFn` checks the backend
|
||||||
|
first and returns `settings.ErrInvalidBackend` BEFORE mutating any controller
|
||||||
|
state on a bad value. On that error → 400, nothing saved, nothing applied,
|
||||||
|
prior wiring intact. On success the handler then persists: `settings.Save` →
|
||||||
|
update in-memory copy → broadcast → 200 with the saved settings. Marvin
|
||||||
|
command and knowledge path are free-form and always accepted (bad values
|
||||||
|
degrade gracefully exactly as today).
|
||||||
|
- `GET /fs/browse?dir=<abspath>` → `{ "dir": "...", "parent": "...",
|
||||||
|
"entries": [ {"name","path","is_dir"} ] }`. Lists subdirectories plus `.md`
|
||||||
|
files. If `dir` is empty/missing, open at the directory of the current
|
||||||
|
knowledge path, else `$HOME`. Rejects a nonexistent/unreadable dir with 400.
|
||||||
|
No filesystem jail beyond OS permissions — localhost-only, single-user daemon,
|
||||||
|
same trust boundary as the rest of the UI.
|
||||||
|
|
||||||
|
**Validation placement (decided):** the backend-name check lives in the injected
|
||||||
|
`applyFn`, which returns a typed `settings.ErrInvalidBackend` (or similar) BEFORE
|
||||||
|
mutating any controller state. The handler maps that error to 400 and skips the
|
||||||
|
save. This keeps `web` free of an `ai` import; `applyFn` (built in `main`) owns
|
||||||
|
all adapter knowledge including what counts as a valid backend.
|
||||||
|
|
||||||
|
### Redundant endpoint removed
|
||||||
|
|
||||||
|
`POST /knowledge/path` becomes redundant (knowledge path now flows through
|
||||||
|
`/settings`). Fold it in and delete the route + `handleKnowledgePath`. The
|
||||||
|
existing `app.js` "change profile" affordance is replaced by the settings
|
||||||
|
overlay's knowledge field.
|
||||||
|
|
||||||
|
### UI (`app.js` / `app.css`)
|
||||||
|
|
||||||
|
- A gear button in the card header toggles a settings overlay over the current
|
||||||
|
card. No routing; reachable from any runtime state.
|
||||||
|
- On open, `GET /settings` populates three controls:
|
||||||
|
- AI backend: `<select>` with claude / codex.
|
||||||
|
- Marvin command: text field.
|
||||||
|
- Knowledge path: text field + **Browse…** button.
|
||||||
|
- **Browse…** opens a lightweight modal driven by `GET /fs/browse`: click a
|
||||||
|
folder to descend, `..` to go up, click a `.md` file to select it back into
|
||||||
|
the path field.
|
||||||
|
- **Save** posts the full object to `/settings`. A 400 renders the error inline
|
||||||
|
and keeps the overlay open. On success the overlay closes; the SSE stream
|
||||||
|
delivers refreshed state.
|
||||||
|
|
||||||
|
## Data flow (save)
|
||||||
|
|
||||||
|
```
|
||||||
|
gear → overlay → edit fields → Save
|
||||||
|
→ POST /settings {ai_backend, marvin_cmd, knowledge_path}
|
||||||
|
→ handler: applyFn(s) validates backend first, mutates only on full success
|
||||||
|
invalid → 400 (no apply, no save)
|
||||||
|
valid → applyFn applied (SetCoach/…, SetTasks, SetKnowledgePath)
|
||||||
|
→ settings.Save(file) → server.current = s → broadcast → 200
|
||||||
|
→ overlay closes, SSE pushes new state
|
||||||
|
```
|
||||||
|
|
||||||
|
## Error handling
|
||||||
|
|
||||||
|
- Invalid AI backend: 400, nothing persisted or applied; prior wiring intact.
|
||||||
|
- Unreadable/missing settings file at startup: treat as first run (seed + save);
|
||||||
|
if the save itself fails, log and continue with the in-memory seed (daemon
|
||||||
|
still starts, matching the existing degrade-not-fail posture).
|
||||||
|
- Bad Marvin command / missing knowledge file: accepted and saved; degrade at
|
||||||
|
fetch/load time exactly as today (no tasks panel / ungrounded coach).
|
||||||
|
- `/fs/browse` on a bad dir: 400 with a short reason; the modal shows it and
|
||||||
|
stays on the last good directory.
|
||||||
|
|
||||||
|
## Testing
|
||||||
|
|
||||||
|
- `internal/settings`: Load/Save round-trip; `DefaultPath`; `SeedFromEnv`
|
||||||
|
reads the three env vars; first-run path (file absent → seed written).
|
||||||
|
- `internal/web`:
|
||||||
|
- `GET /fs/browse` lists entries, filters to dirs + `.md`, exposes correct
|
||||||
|
`parent`, and returns 400 for a nonexistent dir.
|
||||||
|
- `POST /settings` with an invalid backend → 400, applier NOT called, file
|
||||||
|
unchanged (assert via a fake applier returning `ErrInvalidBackend`).
|
||||||
|
- `POST /settings` with valid input → applier called once with the parsed
|
||||||
|
settings and the file written (fake applier records its argument).
|
||||||
|
- The real `applyFn` stays thin in `main` (composition root); its constituent
|
||||||
|
constructors (`ai.NewBackend`, `tasks.NewMarvin`, `knowledge.NewFileSource`,
|
||||||
|
`SetKnowledgePath`) are already covered by their own packages.
|
||||||
|
|
||||||
|
## Files touched
|
||||||
|
|
||||||
|
- `internal/settings/settings.go` (new) + `settings_test.go`
|
||||||
|
- `cmd/antidriftd/main.go` (wiring + applyFn)
|
||||||
|
- `internal/web/settings_handlers.go` (new) + tests
|
||||||
|
- `internal/web/web.go` (register routes, server fields, drop `/knowledge/path`)
|
||||||
|
- `internal/web/static/app.js`, `internal/web/static/app.css` (gear, overlay,
|
||||||
|
browse modal)
|
||||||
|
|
||||||
|
## Out of scope (YAGNI)
|
||||||
|
|
||||||
|
- Port, default timebox, default enforcement level as settings.
|
||||||
|
- Multiple knowledge profiles / profile management.
|
||||||
|
- Any auth on the browse endpoint beyond the existing localhost binding.
|
||||||
|
</content>
|
||||||
|
</invoke>
|
||||||
Reference in New Issue
Block a user