steveandClaude Opus 4.8
41c28592a2
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
193 lines
10 KiB
Markdown
193 lines
10 KiB
Markdown
# CLAUDE.md
|
||
|
||
Working notes for Claude Code on pansy. [DESIGN.md](DESIGN.md) is the architecture
|
||
document and stays authoritative; this file is the operational stuff you'd
|
||
otherwise rediscover every session.
|
||
|
||
## This is a vibe-coded project — say so
|
||
|
||
**pansy is written by an LLM, with a human directing.** That is not a footnote,
|
||
it is a property of the software people should know before they trust it with
|
||
their garden plans.
|
||
|
||
Rules, not preferences:
|
||
|
||
- The README carries a prominent, near-the-top disclosure. It does not get moved
|
||
below the fold, softened into "AI-assisted", or quietly dropped in a rewrite.
|
||
- If you rewrite the README, the disclosure survives the rewrite.
|
||
- Anywhere else the project introduces itself (a landing page, a docs site, a
|
||
package description), it says the same thing.
|
||
|
||
If you find yourself editing that section, the only acceptable direction is
|
||
clearer and more honest, never quieter.
|
||
|
||
## Keep the docs true
|
||
|
||
Docs rot silently and nobody notices until someone follows them and it doesn't
|
||
work. Treat them as part of the change, not follow-up work:
|
||
|
||
- **README.md** — update it in the *same commit* whenever you add or change an
|
||
environment variable, a route worth knowing about, a build or run command, or
|
||
anything in the Docker/Compose example. The env var table and the compose
|
||
snippet are the two things people copy, so they're the two that hurt most when
|
||
they're stale.
|
||
- **DESIGN.md** — update it when the architecture actually changes: a new table,
|
||
a new package, a new API surface, a decision that supersedes one written there.
|
||
Not for every implementation detail.
|
||
- **CLAUDE.md** — this file. Add a convention here the moment you find yourself
|
||
rediscovering it.
|
||
- **Examples must run.** If you change something an example depends on, fix the
|
||
example. A snippet that references an issue number as "once #5 lands" after #5
|
||
has landed is a bug in the docs.
|
||
|
||
When you touch a file, glance at whether the comments around your change are
|
||
still true. Stale comments are worse than none — the next reader believes them.
|
||
|
||
## Build and test
|
||
|
||
pansy is a standalone Go module inside a parent workspace, so **`GOWORK=off` is
|
||
required** or the build picks up sibling modules:
|
||
|
||
```sh
|
||
GOWORK=off go build ./...
|
||
GOWORK=off go test ./...
|
||
cd web && npx tsc --noEmit && npx vitest run && npm run build
|
||
make test # both halves
|
||
make build # web bundle → embed → CGO_ENABLED=0 static binary
|
||
```
|
||
|
||
`gofmt -l internal/` before committing. Note `internal/service/plants_test.go` is
|
||
already unformatted on `main` — leave it alone unless you're touching it, so the
|
||
diff stays about your change.
|
||
|
||
## Architecture in one paragraph
|
||
|
||
`internal/store` (hand-written SQL, `modernc.org/sqlite`, pure Go) →
|
||
`internal/service` (**the seam: every permission check and invariant**) →
|
||
`internal/api` (thin gin handlers: decode, call service, encode). Agent tools in
|
||
`internal/agent` are equally thin adapters over the *same* service methods, so
|
||
they inherit permission enforcement for free. If you are about to put a rule in a
|
||
handler, put it in the service instead.
|
||
|
||
Frontend: React 19 + Vite + Tailwind 4 + TanStack Router/Query, built into
|
||
`internal/webdist/dist` and embedded with `embed.FS`.
|
||
|
||
## Conventions that bite if you miss them
|
||
|
||
- **Everything is centimeters**, stored as SQLite `REAL`. Imperial is a display
|
||
and entry concern only, in `web/src/lib/units.ts`. Two distinct scales live
|
||
there: *dimension* (m/ft — gardens, objects, the garden grid) and *spacing*
|
||
(cm/in — plant spacing, bed grid). Mixing them is what #47 was about.
|
||
- **Optimistic concurrency everywhere.** Every mutable row has `version`; PATCH
|
||
and DELETE carry it; the store returns `(current row, ErrVersionConflict)` on
|
||
mismatch and the API answers 409 with the row under `"current"`.
|
||
- **No-access is `ErrNotFound`, not `ErrForbidden`.** Existence is masked
|
||
deliberately. `ErrForbidden` means "you can see it but may not do that".
|
||
- **Plops (plantings) live in their parent object's local frame**, origin at the
|
||
object's center, `-y` is north. Moving or rotating a bed moves its plants free.
|
||
- **A plop is a clump, not a plant.** `defaultPlopRadius` is `1.5 × spacing`, so a
|
||
plop is three spacings across and holds `π·r²/spacing²` plants. Reasoning about
|
||
fills as if one plop were one plant gets the geometry wrong every time — which
|
||
is how #75 happened: requiring the whole circle inside the bed inset the outer
|
||
row by 1.5 spacings when the horticultural rule is *half* a spacing. Spacing is
|
||
a constraint between neighbouring plants; a bed edge is nobody's neighbour.
|
||
- **Soft removal**: "clear bed" sets `removed_at`; the editor reads
|
||
`removed_at IS NULL`. Hard delete is a different operation.
|
||
- **Migrations** are numbered `.sql` files in `internal/store/migrations/`, run
|
||
at startup, embedded. Never edit one that has shipped.
|
||
- **Every service mutation lands in history** (#48). If you add one, record it —
|
||
see `internal/service/revisions.go`. Multi-row operations pass all their
|
||
changes to a single `record` call so they undo as one unit.
|
||
- **The history write is detached from cancellation on purpose.** `commitScope`
|
||
calls `context.WithoutCancel` — that is not a mistake to tidy up. By the time
|
||
a commit runs, the rows it describes are already written, so cancelling it
|
||
cannot undo anything; it can only leave real changes with no way to undo them.
|
||
This was a live bug twice (#73): a client disconnect mid-request orphaned 18
|
||
plantings. Fixing it per-call-site is how it came back, which is why the rule
|
||
lives in `commitScope` where no caller can forget it.
|
||
|
||
## Testing
|
||
|
||
Match the test to the failure it would catch:
|
||
|
||
- **Anything addressed by its own id needs an API-level test through the router.**
|
||
Service tests can't see a route that was never registered — PATCH/DELETE
|
||
`/journal/:id` once shipped fully implemented, fully unit-tested, and
|
||
completely unreachable.
|
||
- **Watch for fixtures that assert your assumptions instead of the API.** A test
|
||
for the undo message passed because the fixture I wrote populated a field the
|
||
real response leaves empty. If a test builds the thing it's testing against,
|
||
it is checking your mental model, not the system.
|
||
- Some things only real use finds. The agent's whole loop is covered by
|
||
majordomo's scriptable fake provider (`provider/fake`), which is worth using —
|
||
but the three worst v2 bugs all turned up in one live session afterwards.
|
||
|
||
## Workflow
|
||
|
||
Branch → PR → **Gadfly** reviews it automatically → consider every finding and
|
||
fix what's real → merge when the pipeline is green. Do not grade Gadfly findings.
|
||
A push to `main` builds the image and deploys to Komodo; the live instance at
|
||
`pansy.orgrimmar.dudenhoeffer.casa` updates a few minutes later.
|
||
|
||
**Gadfly reviews the PR as opened, not as merged.** The workflow triggers on
|
||
`opened`/`reopened`/`ready_for_review` — deliberately *not* `synchronize` — so
|
||
every commit you push afterwards, including the ones you push in response to
|
||
Gadfly itself, is unreviewed unless you ask. Once you've stopped pushing and
|
||
before you merge, comment **`@gadfly review`** on the PR to re-trigger it. The
|
||
phrase is required, and this is not hypothetical: on #76 the follow-up commit
|
||
was the one that contained a real bug.
|
||
|
||
**A skipped Gadfly run reports success.** A comment without the trigger phrase
|
||
still starts the workflow, which logs `comment does not contain trigger phrase`
|
||
and exits green in ~2 seconds. So "the pipeline is green" does NOT mean "this
|
||
was reviewed". Confirm a re-review actually ran by its **duration** — a real
|
||
pass takes ~10 minutes, a skip takes 2 seconds. Don't look for a new consensus
|
||
comment: Gadfly EDITS its existing status-board and consensus comments in place,
|
||
so their `created_at` stays at the first review and only `updated_at` moves.
|
||
|
||
Workflow- and config-only changes (CI, this file, docs) go straight to `main`
|
||
without the PR dance.
|
||
|
||
Planning happens in Gitea issues first: standalone issues under a tracking epic,
|
||
implemented in later per-issue sessions.
|
||
|
||
## Environment
|
||
|
||
`PANSY_PORT`, `PANSY_DB`, `PANSY_BASE_URL`, `PANSY_REGISTRATION`,
|
||
`PANSY_LOCAL_AUTH`, `PANSY_OIDC_*` — note it's `PANSY_DB` and `PANSY_PORT`, not
|
||
the `_PATH`/`_ADDR` names you might guess. Authentik is the primary IdP;
|
||
OIDC-first with local passwords as fallback.
|
||
|
||
Agent config: `OLLAMA_CLOUD_API_KEY`, `PANSY_AGENT_MODEL` (default
|
||
`ollama-cloud/glm-5.2:cloud`) and `PANSY_AGENT_ENABLED`, set in Komodo. Model
|
||
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
|
||
build, which has no sibling checkout. `executus` is a sibling repo at `../` and
|
||
is not a dependency.
|
||
|
||
The `majordomo` build tag is **gone**. Don't reintroduce it — an untagged CI that
|
||
never compiles the agent is worse than a slightly larger binary.
|